Skip to content

Feat/34/painting-surface-data-simulator-service: 도장표면결함 시뮬레이터 기능 구현#51

Merged
TaeHyunaivle merged 17 commits intomainfrom
feat/34/painting-surface-defect-simulator
Aug 27, 2025
Merged

Feat/34/painting-surface-data-simulator-service: 도장표면결함 시뮬레이터 기능 구현#51
TaeHyunaivle merged 17 commits intomainfrom
feat/34/painting-surface-defect-simulator

Conversation

@leewonwook
Copy link
Member

@leewonwook leewonwook commented Aug 12, 2025

✨ Description

  • 도장 표면 결함 탐지를 위한 실시간 데이터 시뮬레이터 서비스 구현
  • Azure Blob Storage에서 도장 표면 이미지 데이터를 주기적으로 수집하여 결함 탐지 모델 서비스에 전송하고, 결과를 체계적으로 로깅

✅ Task

[x] Azure Blob Storage 연동 서비스 구현
[x] 스케줄러 서비스 구현으로 주기적 데이터 수집
[x] 모델 서비스와의 HTTP 통신 클라이언트 구현
[x] 시뮬레이터 제어 API 구현
[x] 연결 상태 테스트 API 구현
[x] 환경 변수 기반 설정 관리 시스템 구현
[x 로깅 시스템 구현

✅ Test Result

테스트 파일 구성:
test_main.py: 메인 애플리케이션 테스트
test_scheduler_service.py: 스케줄러 서비스 테스트
test_model_client.py: 모델 클라이언트 테스트
test_azure_storage.py: Azure 스토리지 연동 테스트
test_simulator_router.py: 시뮬레이터 라우터 테스트
test_settings.py: 설정 관리 테스트
test_logger.py: 로깅 시스템 테스트
conftest.py: 테스트 설정 및 공통 픽스처
pytest tests/ 명령으로 모든 테스트 통과 확인

📸 Screenshot (선택)

image image image

📌 Etc

  • 환경 변수 .env사용
  • 시뮬레이터 서비스(8012), 도장 표면 모델 서비스(8002)

Summary by CodeRabbit

  • New Features

    • Added Painting Surface Data Simulator service with REST endpoints (start/stop, status, recent logs), health checks, and Dockerized deployment.
  • Documentation

    • Added comprehensive README documenting usage, env vars, API, simulation flow, and Docker instructions.
  • Tests

    • Added extensive async tests, pytest configuration, and a test runner for the new service.
  • Refactor

    • Model service routes and prediction response shape updated; model config loading switched to local files.
  • Chores

    • Added service-specific requirements, removed pinned legacy dev requirements, and simplified CI lint outputs.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 12, 2025

Walkthrough

Adds a new Painting Surface Data Simulator service (FastAPI) with Azure Blob integration, scheduler, model client, structured logging, tests, Dockerfile, and requirements; updates the defect-detection model service startup to use a singleton, adjusts local model config loading and prediction schemas; clears root dev requirements and removes two Flake8 GHA outputs.

Changes

Cohort / File(s) Summary of Changes
Simulator Service — App Core
services/painting-surface-data-simulator-service/app/config/settings.py, .../app/main.py, .../app/routers/simulator_router.py, .../app/routers/test_connection_router.py, .../app/services/azure_storage.py, .../app/services/model_client.py, .../app/services/scheduler_service.py, .../app/utils/logger.py
New Settings (pydantic BaseSettings), FastAPI app with async lifespan, simulator & test routers, async AzureStorageService, PaintingSurfaceModelClient for predictions, SimulatorScheduler (APScheduler) coordinating periodic simulation and backend batch notifications, and AnomalyLogger for structured JSON/error logging.
Simulator Service — Packaging & Docs
services/painting-surface-data-simulator-service/Dockerfile, .../requirements.txt, .../README.md, .../pytest.ini
Added Dockerfile, pinned service requirements.txt, comprehensive README, and pytest.ini for test discovery/config.
Simulator Service — Tests & Helpers
services/painting-surface-data-simulator-service/tests/*
New test suite and fixtures (conftest), unit/async tests for settings, main app, routers, azure storage, model client, scheduler, logger; a test runner script tests/run_tests.py.
Model Service — Startup & Predictors
services/painting-surface-defect-detection-model-service/app/main.py, .../app/routers/predict.py, .../app/models/yolo_model.py
Model service now creates a singleton detection service at startup (load_model/cleanup), exposes get_detection_service(), injects the singleton into predict module, removes /api mount prefix (endpoints at root), adds ImagePathRequest model and extends PredictionResponse fields, and switches some config assets to local file reads.
Model Service — Requirements & Tests
services/painting-surface-defect-detection-model-service/requirements.txt, .../tests/test_model_loader.py
Added urllib3 dependency; moved if __name__ == "__main__" guard in test_model_loader to file bottom (no logic change).
CI/CD Workflow
.github/workflows/ci-cd.yml
Removed two GitHub Actions outputs emitted by the Flake8 step (no other workflow changes).
Repository Root — Dev Deps
requirements-dev.txt
Cleared/deleted pinned development dependencies (file content removed).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Operator as Operator/Client
  participant API as Simulator API (FastAPI)
  participant Sched as SimulatorScheduler
  participant Azure as AzureStorageService
  participant Model as PaintingSurface Backend
  participant Log as AnomalyLogger

  Operator->>API: POST /simulator/start
  API->>Sched: start()
  Sched->>Model: health_check()
  Model-->>Sched: healthy
  Sched-->>API: started

  loop every interval
    Sched->>Azure: simulate_painting_surface_data()
    Azure-->>Sched: {images[], total_images, batch_size} or None
    alt has images
      Sched->>Model: POST /defect-detection/batch-start (metadata)
      Sched->>Model: predict_painting_surface_data(images)
      Model-->>Sched: per-image results + combined
      alt anomaly detected
        Sched->>Log: log_anomaly(...)
      else normal
        Sched->>Log: log_normal_processing(...)
      end
    else no data
      Sched-->>Sched: skip cycle
    end
  end

  Operator->>API: POST /simulator/stop
  API->>Sched: stop()
  Sched-->>API: stopped
Loading
sequenceDiagram
  autonumber
  participant MS as Model Service (FastAPI)
  participant DS as Detection Service (singleton)
  participant Predict as Predict endpoints

  MS->>MS: startup (lifespan)
  MS->>DS: instantiate + await load_model()
  MS->>Predict: set detection_service = DS

  Note over Predict: Endpoints now available at root (e.g., /predict, /predict/file)

  MS->>MS: shutdown
  MS->>DS: await cleanup()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Possibly related issues

Poem

I nibble logs by lantern light,
I fetch the frames, I queue the byte.
Azure carrots, models sing—
I hop, I log each little thing.
If defects pop up, I thump: "Alert!" 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 130c40f and fb5f424.

📒 Files selected for processing (1)
  • services/painting-surface-defect-detection-model-service/requirements.txt (1 hunks)
🔇 Additional comments (1)
services/painting-surface-defect-detection-model-service/requirements.txt (1)

13-13: Confirm urllib3 pin necessity and compatibility

Please verify the following before keeping or removing the urllib3==2.5.0 pin:

  • Check Requests 2.31.0 release notes to see which urllib3 versions it officially supports (see the “Dependencies” or “Changelog” sections).
  • Review urllib3’s security advisories or CVE entries to determine if any vulnerabilities require pinning to ≥2.5.0.
  • If there is no specific CVE or compliance requirement mandating urllib3 2.5.0 (or above), it’s generally safe—and simpler—to let Requests manage its transitive urllib3 dependency.

Once you’ve confirmed:

-services/painting-surface-defect-detection-model-service/requirements.txt
 urllib3==2.5.0

If pinning isn’t needed, consider removing that line:

-urllib3==2.5.0
+# (optional) Let requests manage urllib3 unless a specific CVE requires explicit pinning
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/34/painting-surface-defect-simulator

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link

github-actions bot commented Aug 12, 2025

🤖 Smart FAST 자동 코드 리뷰

📦 변경된 서비스 (1개):

  • painting-surface-data-simulator-service 서비스

🎯 PR 제목에서 서비스 자동 감지됨

  • 제목: Feat/34/painting-surface-data-simulator-service: 도장표면결함 시뮬레이터 기능 구현

📊 전체 검사 결과:

⚠️ 일부 검사에서 문제 발견 - 확인 필요

🧪 테스트 & 코드 품질 결과:

⚠️ 테스트 & 린트 상태 불명

  • 총 테스트: 81개
  • 성공: 59개 ✅
  • 실패: 22개 ❌
  • 커버리지: 0%

🔍 실패한 테스트:

  • painting-surface-data-simulator-service

🔒 보안 스캔 결과:

⚠️ 보안 스캔 상태 불명

  • 보안 이슈가 발견되지 않았습니다 ✅

✅ 검사 체크리스트:

  • ❌ 테스트 실패
  • ✅ 코드 품질 검사 통과
  • ❌ 보안 이슈 발견

🚀 배포 정보:

  • ⚠️ 검사 실패 - 문제 해결 후 배포 가능
    • 🧪 테스트 문제 해결 필요
    • 🔒 보안 이슈 해결 필요
  • 📋 상세 로그는 Actions 탭에서 확인 가능

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 28

🔭 Outside diff range comments (1)
services/painting-surface-data-simulator-service/requirements.txt (1)

1-40: 의존성 버전 보안 검증 결과 및 개선 권고

  • FastAPI
    • 현재: 0.110.0
    • PyPI 최신: 0.116.1 → 보안 패치가 포함된 최신 패치 라인(0.116.x)으로 업데이트 권장
  • aiohttp
    • 현재: 3.9.1
    • PyPI 최신: 3.12.15 → 보안 패치가 포함된 최신 패치 라인(3.12.x)으로 업데이트 권장
  • pip-audit 미설치
    • sandbox 환경에서 pip-audit가 없어 자동 취약점 검사가 실행되지 않았습니다.
    • 로컬 및 CI 환경에 pip install pip-auditpip-audit 실행→보안 리포트 확인 필수

권장 조치

  1. requirements.txt에서 FastAPI·aiohttp 버전을 최신 패치 라인으로 상향 조정
  2. pip-audit 또는 GitHub Dependabot 같은 도구를 CI 파이프라인에 통합하여 주기적 보안 스캔 수행

위 개선 사항 적용 후 다시 검토 부탁드립니다.

🧹 Nitpick comments (37)
services/painting-surface-defect-detection-model-service/tests/test_model_loader.py (1)

787-788: 파일 끝에 불필요한 빈 줄 제거 권장

파일 끝의 빈 줄을 제거하는 것이 좋습니다.

-if __name__ == "__main__":
-    pytest.main([__file__]) 
-
+if __name__ == "__main__":
+    pytest.main([__file__])
services/painting-surface-defect-detection-model-service/app/models/yolo_model.py (1)

110-123: 임계값 설정 파일 로드 시 FileNotFoundError 처리 누락

다른 로드 함수들과 달리 load_threshold_config에서는 FileNotFoundError를 명시적으로 처리하지 않습니다. 일관성을 위해 다른 함수들처럼 명시적으로 처리하는 것이 좋습니다.

 def load_threshold_config(self) -> Dict[str, float]:
     """임계값 설정 파일 로드"""
     try:
         threshold_filename = "thresholds.json"
         # 로컬 파일 경로 사용
         threshold_path = os.path.join(os.path.dirname(__file__), threshold_filename)
         
         with open(threshold_path, "r", encoding="utf-8") as f:
             thresholds = json.load(f)
         
         logger.info("Threshold configuration loaded successfully")
         return thresholds
         
+    except FileNotFoundError:
+        logger.warning(f"Threshold config file '{threshold_filename}' not found, using empty config")
+        return {}
     except Exception as e:
         logger.error(f"Failed to load threshold config: {str(e)}")
         return {}
services/painting-surface-defect-detection-model-service/app/routers/predict.py (1)

40-45: ImagePathRequest 모델이 정의되었지만 사용되지 않음

ImagePathRequest 모델이 정의되었지만 실제로 엔드포인트에서 사용되지 않습니다. 향후 사용 계획이 있다면 TODO 주석을 추가하거나, 사용하지 않을 예정이라면 제거하는 것이 좋습니다.

이미지 경로 기반 예측 엔드포인트를 구현하시겠습니까? 아니면 이 모델을 제거해야 할까요?

services/painting-surface-data-simulator-service/tests/run_tests.py (1)

46-54: Import 방식을 개선하여 static analysis 경고 해결

Static analysis 도구가 지적한 대로, pytesthttpx 모듈을 import하지만 실제로는 사용하지 않습니다. 존재 여부 확인을 위해서는 importlib.util.find_spec를 사용하는 것이 더 적절합니다.

다음과 같이 개선하세요:

    # 테스트 의존성 설치 확인
    print("📦 테스트 의존성 확인 중...")
    try:
-        import pytest
-        import httpx
+        import importlib.util
+        if importlib.util.find_spec("pytest") is None:
+            raise ImportError("pytest")
+        if importlib.util.find_spec("httpx") is None:
+            raise ImportError("httpx")
        print("✅ 필요한 패키지가 설치되어 있습니다.")
    except ImportError as e:
        print(f"❌ 필요한 패키지가 설치되지 않았습니다: {e}")
        print("   다음 명령어로 설치하세요:")
        print("   pip install -r requirements-dev.txt")
        sys.exit(1)
services/painting-surface-data-simulator-service/app/config/settings.py (2)

1-2: 사용하지 않는 import 제거

Static analysis 도구가 지적한 대로 ostyping.Dict 모듈이 import되었지만 실제로 사용되지 않습니다.

사용하지 않는 import를 제거하세요:

-import os
-from typing import Dict
from pydantic_settings import BaseSettings

30-33: 불필요한 property 메서드

model_service_url property가 단순히 painting_model_url 값을 반환하기만 합니다. 추가적인 로직이 없다면 직접 painting_model_url 필드를 사용하는 것이 더 간단합니다.

Property를 제거하고 직접 필드를 사용하거나, 향후 확장성을 위해 유지할지 결정하세요:

-    @property
-    def model_service_url(self) -> str:
-        """도장 표면 결함 감지 모델 서비스 URL"""
-        return self.painting_model_url

다른 코드에서 settings.model_service_url 대신 settings.painting_model_url을 직접 사용하세요.

services/painting-surface-data-simulator-service/README.md (2)

16-31: 코드 블록에 언어 지정 추가

Markdown 린터가 지적한 대로 fenced code block에 언어를 지정하면 가독성이 향상됩니다.

아키텍처 다이어그램과 디렉토리 구조에 적절한 언어 식별자를 추가하세요:

-```
+```text
┌─────────────────┐    ┌──────────────────┐    ┌─────────────────┐
│   Azure Blob    │    │   Simulator      │    │   Painting      │
│   Storage       │◄──►│   Service        │◄──►│   Surface       │
│   (Images)      │    │   (Port 8012)    │    │   Model         │
│                 │    │                  │    │   (Port 8002)   │
└─────────────────┘    └──────────────────┘    └─────────────────┘
                               │
                               ▼
                        ┌──────────────────┐
                        │   Logger         │
                        │   (JSON Files)   │
                        │   - logs/        │
                        │   - errors/      │
                        └──────────────────┘

341-345: 디렉토리 구조 코드 블록에 언어 지정

Line 341의 로그 파일 구조에도 언어 식별자를 추가하세요.

-```
+```text
logs/
├── painting_defect_detections.json    # 결함 탐지 결과 로그
└── painting_errors.json               # 에러 로그
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)

1-4: 사용하지 않는 import 제거 필요

typing.Any, asyncio, azure.core.exceptions.AzureError는 실제로 사용되지 않으므로 제거해야 합니다.

-from typing import List, Dict, Any, Optional
+from typing import List, Dict, Optional
-import asyncio
 from azure.storage.blob.aio import BlobServiceClient
-from azure.core.exceptions import AzureError, ClientAuthenticationError
+from azure.core.exceptions import ClientAuthenticationError
services/painting-surface-data-simulator-service/tests/test_settings.py (1)

1-3: 사용하지 않는 import 제거 필요

pytest는 실제로 사용되지 않으므로 제거해야 합니다.

-import pytest
 import os
 from app.config.settings import Settings
services/painting-surface-data-simulator-service/tests/test_azure_storage.py (1)

1-5: 사용하지 않는 import 제거 필요

asyncioazure.core.exceptions.AzureError는 실제로 사용되지 않으므로 제거해야 합니다.

 import pytest
-import asyncio
 from unittest.mock import AsyncMock, MagicMock, patch
-from azure.core.exceptions import AzureError, ClientAuthenticationError
+from azure.core.exceptions import ClientAuthenticationError
 from app.services.azure_storage import AzureStorageService
services/painting-surface-data-simulator-service/app/routers/test_connection_router.py (2)

4-5: 사용하지 않는 import 제거 필요

settings는 실제로 사용되지 않고, pytest는 라우터 파일에서 필요하지 않습니다.

 from fastapi import APIRouter
 from app.services.model_client import painting_surface_model_client
 from app.services.azure_storage import azure_storage
-from app.config.settings import settings
-import pytest

16-18: Azure Storage 연결 상태 관리 개선 필요

매번 연결하고 끊는 대신, 이미 연결된 상태를 확인하는 것이 효율적일 수 있습니다. 또한 연결이 이미 되어 있는 경우 중복 연결을 피해야 합니다.

     try:
-        await azure_storage.connect()
+        if not azure_storage.client:
+            await azure_storage.connect()
         files = await azure_storage.list_data_files()
-        await azure_storage.disconnect()
+        # 테스트 목적이므로 연결을 끊지 않음 (애플리케이션 수명 주기 동안 유지)
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (3)

1-5: 사용하지 않는 import 제거 필요

datetime는 실제로 사용되지 않으므로 제거해야 합니다.

 import pytest
 import asyncio
 from unittest.mock import AsyncMock, MagicMock, patch
-from datetime import datetime
 from app.services.scheduler_service import SimulatorScheduler

19-21: 사용하지 않는 변수 제거

loop 변수가 할당되었지만 사용되지 않습니다.

             try:
-                loop = asyncio.get_running_loop()
+                asyncio.get_running_loop()
                 asyncio.create_task(self.scheduler.stop())

17-24: 비동기 작업 정리 로직 개선 필요

현재 teardown_method에서 실행 중인 이벤트 루프를 확인하고 있지만, pytest-asyncio를 사용하는 경우 더 안전한 방법이 있습니다.

     def teardown_method(self):
         """각 테스트 메서드 실행 후 정리"""
         if self.scheduler.is_running:
-            # 이벤트 루프가 실행 중일 때만 create_task 사용
-            try:
-                asyncio.get_running_loop()
-                asyncio.create_task(self.scheduler.stop())
-            except RuntimeError:
-                # 이벤트 루프가 실행 중이 아니면 무시
-                pass
+            # pytest-asyncio가 이벤트 루프를 관리하므로
+            # 동기적으로 정리 플래그만 설정
+            self.scheduler.is_running = False
+            if self.scheduler.scheduler:
+                self.scheduler.scheduler.shutdown(wait=False)
services/painting-surface-data-simulator-service/tests/test_logger.py (3)

1-5: 불필요한 import 정리 (Ruff F401).

pytest, tempfile, MagicMock가 사용되지 않습니다. 정리하면 테스트 가독성과 린트 통과에 도움이 됩니다.

다음과 같이 정리하세요:

-import pytest
 import os
 import json
-import tempfile
 from unittest.mock import patch, MagicMock
+from unittest.mock import patch

83-89: 콘솔 출력 검증을 더 엄격하게.

현재는 print가 호출됐는지만 확인합니다. 예상 메시지까지 검증하면 회귀를 더 잘 잡을 수 있습니다.

다음 단언을 추가하세요:

 with patch('builtins.print') as mock_print:
     logger.log_normal_processing(service_name, prediction_result)
     
-    # 콘솔 출력 확인
-    mock_print.assert_called()
+    # 콘솔 출력 확인(예상 메시지)
+    mock_print.assert_any_call(
+        f"✅ NORMAL: {service_name} - 결함 없음 (총 {prediction_result.get('total_count', 'N/A')}개 이미지)"
+    )

14-17: tmp_path 활용 제안(테스트 격리성 개선).

고정 디렉터리명("test-logs") 대신 pytest의 tmp_path를 사용하면 테스트 간 충돌 가능성을 줄일 수 있습니다.

원한다면 setup/teardown 대신 테스트마다 tmp_path로 디렉터리를 생성해 settings 패치를 구성하도록 리팩터링해 드릴 수 있습니다.

services/painting-surface-data-simulator-service/tests/test_model_client.py (1)

2-4: 불필요한 import 정리 (Ruff F401).

asyncio, httpx가 이 파일 내에서 직접 사용되지 않습니다(모킹은 문자열 경로 패치로 충분). 제거하세요.

-import asyncio
 from unittest.mock import AsyncMock, MagicMock, patch
-import httpx
services/painting-surface-data-simulator-service/app/main.py (3)

8-8: 사용되지 않는 import 제거 (Ruff F401).

painting_surface_model_client를 가져오지만 사용하지 않습니다. 제거하세요.

-from app.services.model_client import painting_surface_model_client

44-44: 불필요한 f-string 접두사 제거 (Ruff F541).

문자열에 보간 변수가 없어 f 접두사를 제거하세요.

-    print(f"🎯 대상 서비스: 도장 표면 결함탐지 모델")
+    print("🎯 대상 서비스: 도장 표면 결함탐지 모델")

54-67: 전역 싱글톤 의존성 대신 DI로 전환 권장.

main에서 전역 서비스(azure_storage, simulator_scheduler)를 직접 임포트/사용하고 있습니다. 이전 학습(Learnings)에 따라 FastAPI DI 패턴(app.dependencies에서 setter/getter 제공, 라우터에서 Depends로 주입)으로 전환하면 테스트 용이성과 유지보수성이 개선됩니다.

해당 리팩터를 고려하실까요? 필요하면 기존 simulator_router와 main에 맞춘 DI 초안 패치를 제안드리겠습니다.

services/painting-surface-data-simulator-service/tests/test_simulator_router.py (2)

4-9: 불필요한 import 정리 (Ruff F401).

사용되지 않는 MagicMock, TestClient, simulator_scheduler, painting_surface_model_client를 제거하세요.

-from unittest.mock import AsyncMock, MagicMock, patch
-from fastapi.testclient import TestClient
+from unittest.mock import AsyncMock, patch
 from fastapi import HTTPException
-from app.routers.simulator_router import get_simulator_status, start_simulator, stop_simulator, get_recent_logs
-from app.services.scheduler_service import simulator_scheduler
-from app.services.model_client import painting_surface_model_client
+from app.routers.simulator_router import get_simulator_status, start_simulator, stop_simulator, get_recent_logs

216-223: 중첩 with를 단일 with로 합치기 (SIM117).

컨텍스트 매니저를 하나로 합치면 더 간결하고 파이썬 스타일 가이드에 부합합니다.

-        with patch('os.path.exists', return_value=True):
-            with patch('builtins.open', side_effect=PermissionError("Permission denied")):
-                # 로그 조회 시도 (예외 발생)
-                with pytest.raises(HTTPException) as exc_info:
-                    await get_recent_logs()
+        with patch('os.path.exists', return_value=True), \
+             patch('builtins.open', side_effect=PermissionError("Permission denied")):
+            # 로그 조회 시도 (예외 발생)
+            with pytest.raises(HTTPException) as exc_info:
+                await get_recent_logs()
services/painting-surface-data-simulator-service/tests/conftest.py (2)

19-25: 중복 asyncio 마커 적용 가능성.

파일 상단의 pytest_collection_modifyitems에서 코루틴을 자동으로 asyncio 마킹합니다. 개별 테스트에서도 @pytest.mark.asyncio를 사용하므로 중복입니다. 한 곳으로 일원화하는 것을 권장합니다.

원하시면 컬렉션 훅을 제거하고 테스트 파일의 명시적 마커만 유지하도록 변경하는 패치를 드리겠습니다.


35-52: Settings 모킹 일관성 확인.

mock.model_service_url과 painting_model_url 모두 설정되어 있습니다. app/config/settings.py의 model_service_url 프로퍼티가 painting_model_url을 반환하므로 둘 중 하나만으로도 충분합니다. 한쪽으로 정리하면 혼동을 줄일 수 있습니다.

-    mock.painting_model_url = "http://test-model:8002"
-    mock.model_service_url = "http://test-model:8002"  # 프로퍼티 추가
+    mock.painting_model_url = "http://test-model:8002"

또는 반대로 model_service_url만 사용하도록 테스트/클라이언트 초기화 경로를 통일하세요.

services/painting-surface-data-simulator-service/tests/test_main.py (3)

2-3: 사용하지 않는 import 제거

asyncioMagicMock이 import되었지만 사용되지 않습니다.

-import asyncio
-from unittest.mock import AsyncMock, MagicMock, patch
+from unittest.mock import AsyncMock, patch

38-44: 테스트 응답 코드 검증 개선 필요

시뮬레이터 라우터 테스트에서 여러 상태 코드를 허용하는 것은 테스트의 명확성을 떨어뜨립니다.

 def test_simulator_router_included(self):
     """시뮬레이터 라우터 포함 여부 테스트"""
     response = self.client.get("/simulator/status")
     
-    # 라우터가 포함되어 있다면 200 또는 적절한 응답을 받아야 함
-    # 실제 구현에 따라 응답이 달라질 수 있음
-    assert response.status_code in [200, 404, 500]
+    # 라우터가 포함되어 있다면 200 응답을 받아야 함
+    assert response.status_code == 200

46-52: 테스트 응답 코드 검증 개선 필요

테스트 연결 라우터 테스트에서도 명확한 예상 응답 코드를 지정해야 합니다.

 def test_test_connection_router_included(self):
     """테스트 연결 라우터 포함 여부 테스트"""
     response = self.client.post("/test/azure-storage-connection")
     
-    # 라우터가 포함되어 있다면 적절한 응답을 받아야 함
-    # 실제 구현에 따라 응답이 달라질 수 있음
-    assert response.status_code in [200, 404, 500]
+    # Azure 연결이 설정되지 않은 경우 500 응답 예상
+    assert response.status_code == 500
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (2)

44-44: 불필요한 f-string 접두사 제거

플레이스홀더가 없는 문자열에 f-string 접두사가 사용되었습니다.

-    print(f"📊 대상 서비스: 도장 표면 결함탐지 모델")
+    print("📊 대상 서비스: 도장 표면 결함탐지 모델")

73-73: 불필요한 f-string 접두사 제거

플레이스홀더가 없는 문자열에 f-string 접두사가 사용되었습니다.

-    print(f"📈 활성 서비스: 1/1")
+    print("📈 활성 서비스: 1/1")
services/painting-surface-data-simulator-service/app/services/model_client.py (5)

5-5: 사용하지 않는 import 제거

datetime이 import되었지만 사용되지 않습니다.

-from datetime import datetime

20-20: 불필요한 f-string 접두사 제거

플레이스홀더가 없는 문자열에 f-string 접두사가 사용되었습니다.

-    print(f"❌ Painting Surface 서비스 URL을 찾을 수 없습니다.")
+    print("❌ Painting Surface 서비스 URL을 찾을 수 없습니다.")

29-29: 불필요한 f-string 접두사 제거

플레이스홀더가 없는 문자열에 f-string 접두사가 사용되었습니다.

-    print(f"🎨 도장 표면 이미지 예측 요청...")
+    print("🎨 도장 표면 이미지 예측 요청...")

76-79: 조건부 할당 간소화

if-else 블록을 삼항 연산자로 간소화할 수 있습니다.

-    if defect_count > 0:
-        final_status = "anomaly"
-    else:
-        final_status = "normal"
+    final_status = "anomaly" if defect_count > 0 else "normal"

165-165: 불필요한 f-string 접두사 제거

플레이스홀더가 없는 문자열에 f-string 접두사가 사용되었습니다.

-    print(f"   ✅ 결함 없음 - 정상 상태")
+    print("   ✅ 결함 없음 - 정상 상태")
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c5b690 and 13796e3.

📒 Files selected for processing (25)
  • requirements-dev.txt (0 hunks)
  • services/painting-surface-data-simulator-service/Dockerfile (1 hunks)
  • services/painting-surface-data-simulator-service/README.md (1 hunks)
  • services/painting-surface-data-simulator-service/app/config/settings.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/main.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/routers/simulator_router.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/routers/test_connection_router.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/azure_storage.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/model_client.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/scheduler_service.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/utils/logger.py (1 hunks)
  • services/painting-surface-data-simulator-service/requirements.txt (1 hunks)
  • services/painting-surface-data-simulator-service/tests/conftest.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/run_tests.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_azure_storage.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_logger.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_main.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_model_client.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_settings.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_simulator_router.py (1 hunks)
  • services/painting-surface-defect-detection-model-service/app/main.py (3 hunks)
  • services/painting-surface-defect-detection-model-service/app/models/yolo_model.py (3 hunks)
  • services/painting-surface-defect-detection-model-service/app/routers/predict.py (4 hunks)
  • services/painting-surface-defect-detection-model-service/tests/test_model_loader.py (1 hunks)
💤 Files with no reviewable changes (1)
  • requirements-dev.txt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T07:44:19.852Z
Learnt from: doyooon
PR: smart-factory-team/smart_FAST#30
File: services/painting-surface-defect-detection-model-service/app/main.py:8-26
Timestamp: 2025-08-08T07:44:19.852Z
Learning: For the painting surface defect service, avoid using global variables for dependency sharing between main and routers. Prefer FastAPI DI with a dedicated app.dependencies module exposing set_detection_service()/get_detection_service(), and inject the service via Depends in router endpoints.

Applied to files:

  • services/painting-surface-data-simulator-service/app/main.py
  • services/painting-surface-defect-detection-model-service/app/main.py
🧬 Code Graph Analysis (12)
services/painting-surface-data-simulator-service/app/main.py (2)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)
  • connect (17-36)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (2)
  • stop (51-60)
  • get_status (132-140)
services/painting-surface-data-simulator-service/tests/test_main.py (8)
services/painting-surface-data-simulator-service/tests/test_logger.py (1)
  • setup_method (12-16)
services/painting-surface-data-simulator-service/tests/test_azure_storage.py (1)
  • setup_method (11-15)
services/painting-surface-data-simulator-service/tests/test_model_client.py (1)
  • setup_method (11-15)
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (1)
  • setup_method (11-13)
services/painting-surface-data-simulator-service/tests/test_simulator_router.py (1)
  • setup_method (15-21)
services/painting-surface-data-simulator-service/tests/conftest.py (3)
  • mock_azure_storage (56-67)
  • mock_settings (36-52)
  • mock_scheduler (84-95)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)
  • connect (17-36)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (1)
  • stop (51-60)
services/painting-surface-data-simulator-service/tests/conftest.py (4)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • model_service_url (31-33)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (5)
  • connect (17-36)
  • disconnect (53-56)
  • list_data_files (58-86)
  • read_image_data (88-108)
  • simulate_painting_surface_data (126-158)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)
  • predict_painting_surface_data (17-62)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (3)
  • start (15-49)
  • stop (51-60)
  • get_status (132-140)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (4)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (2)
  • disconnect (53-56)
  • simulate_painting_surface_data (126-158)
services/painting-surface-data-simulator-service/app/main.py (1)
  • health_check (87-89)
services/painting-surface-data-simulator-service/app/services/model_client.py (2)
  • health_check (89-101)
  • predict_painting_surface_data (17-62)
services/painting-surface-data-simulator-service/app/utils/logger.py (3)
  • log_anomaly (15-34)
  • log_normal_processing (36-39)
  • log_error (41-55)
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (4)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (6)
  • SimulatorScheduler (10-140)
  • stop (51-60)
  • start (15-49)
  • get_status (132-140)
  • _simulate_data_collection (76-130)
  • _initial_health_check (62-74)
services/painting-surface-data-simulator-service/tests/conftest.py (3)
  • mock_azure_storage (56-67)
  • mock_model_client (71-80)
  • mock_settings (36-52)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (3)
  • connect (17-36)
  • disconnect (53-56)
  • simulate_painting_surface_data (126-158)
services/painting-surface-data-simulator-service/app/utils/logger.py (3)
  • log_normal_processing (36-39)
  • log_anomaly (15-34)
  • log_error (41-55)
services/painting-surface-data-simulator-service/app/routers/test_connection_router.py (3)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (3)
  • connect (17-36)
  • list_data_files (58-86)
  • disconnect (53-56)
services/painting-surface-data-simulator-service/app/main.py (1)
  • health_check (87-89)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)
  • health_check (89-101)
services/painting-surface-data-simulator-service/tests/test_settings.py (1)
services/painting-surface-data-simulator-service/app/config/settings.py (2)
  • Settings (6-38)
  • model_service_url (31-33)
services/painting-surface-data-simulator-service/tests/test_azure_storage.py (1)
services/painting-surface-data-simulator-service/tests/conftest.py (1)
  • mock_settings (36-52)
services/painting-surface-data-simulator-service/app/routers/simulator_router.py (2)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (3)
  • get_status (132-140)
  • start (15-49)
  • stop (51-60)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)
  • health_check (89-101)
services/painting-surface-data-simulator-service/app/services/model_client.py (3)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • model_service_url (31-33)
services/painting-surface-data-simulator-service/app/main.py (1)
  • health_check (87-89)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)
  • read_image_data (88-108)
services/painting-surface-data-simulator-service/tests/test_model_client.py (1)
services/painting-surface-data-simulator-service/tests/conftest.py (1)
  • mock_settings (36-52)
services/painting-surface-defect-detection-model-service/app/main.py (2)
services/painting-surface-defect-detection-model-service/app/services/inference.py (4)
  • PaintingSurfaceDefectDetectionService (36-208)
  • load_model (49-75)
  • predict (100-135)
  • cleanup (173-195)
services/painting-surface-defect-detection-model-service/app/routers/predict.py (2)
  • get_detection_service (56-60)
  • predict (64-101)
🪛 Ruff (0.12.2)
services/painting-surface-data-simulator-service/app/main.py

8-8: app.services.model_client.painting_surface_model_client imported but unused

Remove unused import: app.services.model_client.painting_surface_model_client

(F401)


44-44: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/tests/run_tests.py

47-47: pytest imported but unused; consider using importlib.util.find_spec to test for availability

(F401)


48-48: httpx imported but unused; consider using importlib.util.find_spec to test for availability

(F401)

services/painting-surface-data-simulator-service/tests/test_main.py

2-2: asyncio imported but unused

Remove unused import: asyncio

(F401)


3-3: unittest.mock.MagicMock imported but unused

Remove unused import: unittest.mock.MagicMock

(F401)

services/painting-surface-data-simulator-service/app/services/azure_storage.py

1-1: typing.Any imported but unused

Remove unused import: typing.Any

(F401)


2-2: asyncio imported but unused

Remove unused import: asyncio

(F401)


4-4: azure.core.exceptions.AzureError imported but unused

Remove unused import: azure.core.exceptions.AzureError

(F401)

services/painting-surface-data-simulator-service/app/services/scheduler_service.py

44-44: f-string without any placeholders

Remove extraneous f prefix

(F541)


73-73: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/tests/test_simulator_router.py

4-4: unittest.mock.MagicMock imported but unused

Remove unused import: unittest.mock.MagicMock

(F401)


5-5: fastapi.testclient.TestClient imported but unused

Remove unused import: fastapi.testclient.TestClient

(F401)


8-8: app.services.scheduler_service.simulator_scheduler imported but unused

Remove unused import: app.services.scheduler_service.simulator_scheduler

(F401)


9-9: app.services.model_client.painting_surface_model_client imported but unused

Remove unused import: app.services.model_client.painting_surface_model_client

(F401)


216-217: Use a single with statement with multiple contexts instead of nested with statements

(SIM117)

services/painting-surface-data-simulator-service/tests/test_scheduler_service.py

4-4: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)


20-20: Local variable loop is assigned to but never used

Remove assignment to unused variable loop

(F841)

services/painting-surface-data-simulator-service/app/routers/test_connection_router.py

4-4: app.config.settings.settings imported but unused

Remove unused import: app.config.settings.settings

(F401)

services/painting-surface-data-simulator-service/tests/test_settings.py

1-1: pytest imported but unused

Remove unused import: pytest

(F401)

services/painting-surface-data-simulator-service/tests/test_azure_storage.py

2-2: asyncio imported but unused

Remove unused import: asyncio

(F401)


4-4: azure.core.exceptions.AzureError imported but unused

Remove unused import: azure.core.exceptions.AzureError

(F401)

services/painting-surface-data-simulator-service/app/routers/simulator_router.py

35-35: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


48-48: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


68-68: Do not use bare except

(E722)


77-77: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

services/painting-surface-data-simulator-service/app/services/model_client.py

5-5: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)


20-20: f-string without any placeholders

Remove extraneous f prefix

(F541)


29-29: f-string without any placeholders

Remove extraneous f prefix

(F541)


76-79: Use ternary operator final_status = "anomaly" if defect_count > 0 else "normal" instead of if-else-block

Replace if-else-block with final_status = "anomaly" if defect_count > 0 else "normal"

(SIM108)


100-100: Do not use bare except

(E722)


165-165: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/tests/test_model_client.py

2-2: asyncio imported but unused

Remove unused import: asyncio

(F401)


4-4: httpx imported but unused

Remove unused import: httpx

(F401)


53-53: Local variable mock_log is assigned to but never used

Remove assignment to unused variable mock_log

(F841)

services/painting-surface-data-simulator-service/app/config/settings.py

1-1: os imported but unused

Remove unused import: os

(F401)


2-2: typing.Dict imported but unused

Remove unused import: typing.Dict

(F401)

services/painting-surface-data-simulator-service/tests/test_logger.py

1-1: pytest imported but unused

Remove unused import: pytest

(F401)


4-4: tempfile imported but unused

Remove unused import: tempfile

(F401)


5-5: unittest.mock.MagicMock imported but unused

Remove unused import: unittest.mock.MagicMock

(F401)

services/painting-surface-defect-detection-model-service/app/main.py

6-6: typing.Annotated imported but unused

Remove unused import: typing.Annotated

(F401)


7-7: fastapi.Depends imported but unused

Remove unused import: fastapi.Depends

(F401)

🪛 markdownlint-cli2 (0.17.2)
services/painting-surface-data-simulator-service/README.md

16-16: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


341-341: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (10)
services/painting-surface-defect-detection-model-service/tests/test_model_loader.py (1)

785-786: 코드가 정상적으로 작동합니다

테스트 로직이 올바르게 구현되어 있습니다. 첫 번째 로더의 속성을 수정해도 두 번째 로더 인스턴스가 영향받지 않는지 확인하는 격리 테스트가 적절히 작성되었습니다.

services/painting-surface-defect-detection-model-service/app/models/yolo_model.py (2)

91-106: 클래스 매핑 파일의 존재 여부 확인 필요

class_mapping.json 파일도 로컬 경로에서 로드하도록 변경되었습니다. 파일이 없으면 기본 매핑을 사용하지만, 프로덕션 환경에서는 이 파일이 반드시 필요할 수 있습니다.


72-86: model_config.json 파일 존재 여부 확인 필요
FileNotFoundError 발생 시 빈 딕셔너리를 반환하므로, 실제로 해당 파일이 리포지토리 및 배포 아티팩트에 포함되었는지 검증해 주세요.

  • 리포지토리 루트에서 model_config.json 검색 및 경로 확인
  • 배포 패키지나 컨테이너 이미지에 설정 파일이 포함되었는지 점검
services/painting-surface-data-simulator-service/Dockerfile (1)

1-29: Docker 이미지 설정이 잘 구성되어 있습니다.

Python 3.10 기반의 FastAPI 애플리케이션을 위한 적절한 Dockerfile 구성입니다. 포트 8012 노출과 uvicorn 서버 설정이 PR 목표와 일치합니다.

services/painting-surface-data-simulator-service/README.md (1)

1-405: 포괄적이고 잘 구조화된 문서

이 README는 새로운 서비스에 대한 포괄적인 문서를 제공합니다. 설치, 구성, API 엔드포인트, 모니터링 등 모든 주요 측면을 다루고 있어 개발자와 운영팀 모두에게 유용합니다. 한국어로 작성되어 팀의 접근성도 우수합니다.

services/painting-surface-data-simulator-service/app/main.py (1)

31-38: Azure 연결 실패 시 부팅 지속 정책 확인.

연결 실패를 로깅만 하고 서비스는 계속 기동합니다. 설계 의도가 “연결 실패에도 시뮬레이터 기동”이라면 OK입니다. 반대로 하드 실패가 맞다면 예외를 재전달해 프로세스를 종료해야 합니다.

정책을 확정해 주세요. 필요 시 이 구간을 fail-fast 또는 재시도(backoff 포함)로 조정하는 패치를 제공할 수 있습니다.

services/painting-surface-data-simulator-service/tests/test_simulator_router.py (1)

30-53: 상태 응답 키 이름 불일치 가능성 주의.

mock_scheduler.get_status가 반환하는 키는 테스트에서 running/started_at/jobs입니다. 실제 구현(app/services/scheduler_service.py) 요약에 따르면 get_status는 is_running/interval_minutes/next_run/target_service를 반환합니다. 라우터에서 별도 매핑이 없다면 불일치로 인한 실패 가능성이 있습니다.

라우터 구현에서 키 변환을 수행하는지 확인 바랍니다. 필요 시 응답 스키마 정합화 패치를 제안드릴 수 있습니다.

services/painting-surface-data-simulator-service/tests/conftest.py (1)

27-33: event_loop 스코프 검토.

기본 function 스코프는 테스트마다 루프를 생성/종료합니다. 성능과 안정성 사이의 트레이드오프가 있습니다. 세션 스코프로 올리면 성능은 좋아지나 공유 상태 이슈가 생길 수 있습니다. 프로젝트 정책에 맞게 스코프를 확정하세요.

원하시면 session 스코프 전환 및 충돌 방지를 위한 가이드(예: fixture 격리) 제공 가능합니다.

services/painting-surface-data-simulator-service/tests/test_main.py (1)

15-27: 테스트 메서드 구조 양호

루트 엔드포인트 테스트가 모든 필수 필드를 적절히 검증하고 있습니다.

services/painting-surface-data-simulator-service/app/services/scheduler_service.py (1)

76-130: 데이터 수집 메서드 구현 우수

비동기 데이터 수집 및 예측 로직이 잘 구조화되어 있으며, 적절한 오류 처리와 로깅이 구현되어 있습니다.


class Settings(BaseSettings):
# Azure Storage 설정
azure_connection_string: str
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

필수 환경 변수에 대한 검증 로직 필요

azure_connection_string은 필수 필드이지만 기본값이 없습니다. 환경 변수가 설정되지 않은 경우 명확한 오류 메시지를 제공하는 검증 로직을 추가하는 것이 좋습니다.

필드에 대한 검증과 명확한 오류 메시지를 추가하세요:

+from pydantic import field_validator

class Settings(BaseSettings):
    # Azure Storage 설정
    azure_connection_string: str
+    
+    @field_validator('azure_connection_string')
+    @classmethod
+    def validate_azure_connection_string(cls, v):
+        if not v or v.strip() == "":
+            raise ValueError(
+                "AZURE_CONNECTION_STRING 환경 변수가 설정되지 않았습니다. "
+                ".env 파일에서 Azure Storage 연결 문자열을 설정해주세요."
+            )
+        return v
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
azure_connection_string: str
from pydantic import BaseSettings, field_validator
class Settings(BaseSettings):
# Azure Storage 설정
azure_connection_string: str
@field_validator("azure_connection_string")
@classmethod
def validate_azure_connection_string(cls, v):
if not v or v.strip() == "":
raise ValueError(
"AZURE_CONNECTION_STRING 환경 변수가 설정되지 않았습니다. "
".env 파일에서 Azure Storage 연결 문자열을 설정해주세요."
)
return v
🤖 Prompt for AI Agents
In services/painting-surface-data-simulator-service/app/config/settings.py
around line 8, the azure_connection_string field is required but has no
validation; add logic so the application fails fast with a clear message when
the environment variable is missing or empty. Implement validation when loading
settings (e.g., with pydantic validator or explicit check) to raise a
descriptive error like "AZURE_CONNECTION_STRING is required" if the value is
None or blank, and ensure the error is raised early during startup so missing
config surfaces immediately.

"status": simulator_scheduler.get_status()
}
except Exception as e:
raise HTTPException(status_code=500, detail=f"시뮬레이터 시작 실패: {str(e)}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

예외 체인 처리 개선 필요

예외를 다시 발생시킬 때 원인 예외를 명시하여 디버깅을 용이하게 해야 합니다.

-        raise HTTPException(status_code=500, detail=f"시뮬레이터 시작 실패: {str(e)}")
+        raise HTTPException(status_code=500, detail=f"시뮬레이터 시작 실패: {str(e)}") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
raise HTTPException(status_code=500, detail=f"시뮬레이터 시작 실패: {str(e)}")
raise HTTPException(status_code=500, detail=f"시뮬레이터 시작 실패: {str(e)}") from e
🧰 Tools
🪛 Ruff (0.12.2)

35-35: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents
In
services/painting-surface-data-simulator-service/app/routers/simulator_router.py
around line 35, the code raises an HTTPException without preserving the original
exception context; update the raise to chain the original exception (use "raise
HTTPException(status_code=500, detail=f\"시뮬레이터 시작 실패: {str(e)}\") from e") so
the original traceback is preserved for debugging and optionally add a brief log
statement emitting the original exception before re-raising.

"status": simulator_scheduler.get_status()
}
except Exception as e:
raise HTTPException(status_code=500, detail=f"시뮬레이터 중지 실패: {str(e)}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

예외 체인 처리 개선 필요

예외를 다시 발생시킬 때 원인 예외를 명시해야 합니다.

-        raise HTTPException(status_code=500, detail=f"시뮬레이터 중지 실패: {str(e)}")
+        raise HTTPException(status_code=500, detail=f"시뮬레이터 중지 실패: {str(e)}") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
raise HTTPException(status_code=500, detail=f"시뮬레이터 중지 실패: {str(e)}")
raise HTTPException(status_code=500, detail=f"시뮬레이터 중지 실패: {str(e)}") from e
🧰 Tools
🪛 Ruff (0.12.2)

48-48: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents
In
services/painting-surface-data-simulator-service/app/routers/simulator_router.py
around line 48, the HTTPException is raised without preserving the original
exception context; change the raise to chain the original exception (use "raise
HTTPException(status_code=500, detail=..., ) from e") so the cause is preserved
for debugging and tracebacks.

Comment on lines +68 to +69
except:
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

너무 광범위한 예외 처리

bare except 사용은 예상치 못한 오류를 숨길 수 있습니다. 구체적인 예외 타입을 명시해야 합니다.

-                except:
-                    continue
+                except (json.JSONDecodeError, ValueError):
+                    continue
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
except:
continue
except (json.JSONDecodeError, ValueError):
continue
🧰 Tools
🪛 Ruff (0.12.2)

68-68: Do not use bare except

(E722)


return {
"logs": logs,
"total_count": len(lines) if 'lines' in locals() else 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

잠재적 참조 오류 위험

lines 변수가 예외 발생 시 정의되지 않을 수 있습니다. 안전한 처리가 필요합니다.

-            "total_count": len(lines) if 'lines' in locals() else 0
+            "total_count": len(logs) if logs else 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"total_count": len(lines) if 'lines' in locals() else 0
"total_count": len(logs) if logs else 0
🤖 Prompt for AI Agents
In
services/painting-surface-data-simulator-service/app/routers/simulator_router.py
around line 73, the code uses len(lines) in the response but lines may be
undefined if an exception occurs; initialize lines to an empty list before the
try/except that populates it (or retrieve it safely via locals().get('lines',
[])) and then use that safe value when building the response so the reference
cannot raise a NameError.

Comment on lines +6 to +7
from typing import Annotated, Optional
from fastapi import Depends
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

사용하지 않는 임포트 제거

AnnotatedDepends가 임포트되었지만 사용되지 않습니다.

-from typing import Annotated, Optional
-from fastapi import Depends
+from typing import Optional
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from typing import Annotated, Optional
from fastapi import Depends
-from typing import Annotated, Optional
-from fastapi import Depends
+from typing import Optional
🧰 Tools
🪛 Ruff (0.12.2)

6-6: typing.Annotated imported but unused

Remove unused import: typing.Annotated

(F401)


7-7: fastapi.Depends imported but unused

Remove unused import: fastapi.Depends

(F401)

🤖 Prompt for AI Agents
In services/painting-surface-defect-detection-model-service/app/main.py around
lines 6-7, the imports Annotated and Depends are unused; remove them from the
import statements so only actually used names are imported (e.g., keep Optional
and import FastAPI/other used items), and run a quick linter/pytest to confirm
no references remain.

Comment on lines +13 to +16
async def get_detection_service() -> PaintingSurfaceDefectDetectionService:
if _detection_service is None:
raise RuntimeError("Detection service not initialized")
return _detection_service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

기존 학습 내용과 불일치: 전역 변수 사용

이전 PR #30에서 전역 변수 대신 FastAPI DI(Dependency Injection)를 사용하라는 피드백이 있었습니다. 현재 구현은 여전히 전역 변수 _detection_service를 사용하고 있습니다. app.dependencies 모듈을 만들어 set_detection_service()/get_detection_service()를 노출하고, 라우터 엔드포인트에서 Depends를 통해 서비스를 주입하는 방식으로 변경하는 것이 권장됩니다.

FastAPI의 의존성 주입 패턴을 사용하여 전역 변수를 제거하는 리팩토링을 도와드릴까요?

🤖 Prompt for AI Agents
In services/painting-surface-defect-detection-model-service/app/main.py around
lines 13-16, the code still relies on the global _detection_service; replace
this with a FastAPI DI pattern by creating a new module app.dependencies that
exposes set_detection_service(app, service) and get_detection_service()
dependency functions (get_detection_service should be a FastAPI dependency
callable that reads the service from app.state or a closure and raises
RuntimeError if missing), move service initialization into the FastAPI startup
event and call set_detection_service there, and update all router endpoints to
accept the service via Depends(get_detection_service) instead of referencing the
global variable, then remove the _detection_service global from main.py.

Comment on lines +26 to +28
# predict.py의 전역 변수 설정
import app.routers.predict as predict_module
predict_module.detection_service = _detection_service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

안티패턴: predict 모듈의 전역 변수 직접 수정

predict 모듈의 전역 변수를 직접 수정하는 것은 좋지 않은 패턴입니다. 대신 FastAPI의 의존성 주입 시스템을 사용해야 합니다.

-        # predict.py의 전역 변수 설정
-        import app.routers.predict as predict_module
-        predict_module.detection_service = _detection_service
+        # FastAPI 의존성 주입을 통해 서비스 제공
+        # predict.py에서 Depends(get_detection_service) 사용
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# predict.py의 전역 변수 설정
import app.routers.predict as predict_module
predict_module.detection_service = _detection_service
# FastAPI 의존성 주입을 통해 서비스 제공
# predict.py에서 Depends(get_detection_service) 사용

Comment on lines +63 to +70
if __name__ == "__main__":
# 시뮬레이터에서 접근할 수 있도록 포트 8002로 설정
uvicorn.run(
"main:app",
host="0.0.0.0", # 모든 인터페이스에서 접근 가능
port=8002,
reload=True
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

프로덕션 환경에서 reload=True 사용 주의

reload=True 옵션은 개발 환경에서만 사용해야 합니다. 프로덕션 환경에서는 성능 저하와 불안정성을 야기할 수 있습니다.

 if __name__ == "__main__":
     # 시뮬레이터에서 접근할 수 있도록 포트 8002로 설정
+    import os
+    is_dev = os.getenv("ENVIRONMENT", "development") == "development"
     uvicorn.run(
         "main:app",
         host="0.0.0.0",  # 모든 인터페이스에서 접근 가능
         port=8002,
-        reload=True
+        reload=is_dev
     )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if __name__ == "__main__":
# 시뮬레이터에서 접근할 수 있도록 포트 8002로 설정
uvicorn.run(
"main:app",
host="0.0.0.0", # 모든 인터페이스에서 접근 가능
port=8002,
reload=True
)
if __name__ == "__main__":
# 시뮬레이터에서 접근할 수 있도록 포트 8002로 설정
import os
is_dev = os.getenv("ENVIRONMENT", "development") == "development"
uvicorn.run(
"main:app",
host="0.0.0.0", # 모든 인터페이스에서 접근 가능
port=8002,
reload=is_dev
)
🤖 Prompt for AI Agents
In services/painting-surface-defect-detection-model-service/app/main.py around
lines 63 to 70, the uvicorn.run call sets reload=True which is only suitable for
development; remove reload=True (or make it conditional on an environment
variable like DEBUG/ENV) so production runs start without the auto-reloader—set
reload to False or omit the argument, and document/use an env flag to enable
reload only when running in development.



@router.post("/predict", response_model=PredictionResponse)
@router.post("/predict/file", response_model=PredictionResponse)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

엔드포인트 경로 변경 확인

예측 엔드포인트 경로가 /predict/file로 변경되었습니다. API 클라이언트들이 이 변경사항을 인지하고 있는지 확인이 필요합니다.


🏁 Script executed:

#!/bin/bash
# 이전 엔드포인트 경로를 사용하는 코드가 있는지 확인
rg -A 2 'predict["\047]' --type py services/

Length of output: 237


다음 스크립트로 하드코딩된 이전 엔드포인트 /predict 사용 여부를 재확인해주세요:


🏁 Script executed:

#!/bin/bash
# 하드코딩된 '/predict' 경로 검색 (services/ 디렉터리 전체)
rg --fixed-strings '/predict' --type py -n services/ -C2

Length of output: 100971


테스트에서 파일 업로드 엔드포인트 경로 및 Base64 엔드포인트 경로 수정 필요

파일 업로드용 엔드포인트를 /predict/file로 변경했으므로, 기존 테스트 및 메인 검증 코드에 남아 있는 /api/predict/api/predict/base64 경로를 아래와 같이 업데이트해야 합니다:

  • services/painting-surface-defect-detection-model-service/tests/test_predict_router.py
    • 파일 업로드 방식:
    – 변경 전: client.post("/api/predict", …)
    – 변경 후: client.post("/api/predict/file", …)
    • Base64 방식:
    – 변경 전: client.post("/api/predict/base64", …)
    – 변경 후: client.post("/api/predict", …)

  • services/painting-surface-defect-detection-model-service/tests/test_main.py
    • predict_path: /api/predict/api/predict/file
    • predict_base64_path: /api/predict/base64/api/predict

참고: painting-surface-data-simulator-service 쪽 클라이언트 코드(model_client.py)는 이미 /predict/file을 사용하고 있으므로, 테스트와 메인 검증 코드만 경로 일관성을 맞춰주세요.

🤖 Prompt for AI Agents
In
services/painting-surface-defect-detection-model-service/app/routers/predict.py
around line 63 the file-upload endpoint was changed to "/predict/file", so
update all test and main paths to match: in
services/painting-surface-defect-detection-model-service/tests/test_predict_router.py
change client.post("/api/predict", ...) to client.post("/api/predict/file", ...)
for the file upload test and change client.post("/api/predict/base64", ...) to
client.post("/api/predict", ...) for the base64 test; in
services/painting-surface-defect-detection-model-service/tests/test_main.py set
predict_path from "/api/predict" to "/api/predict/file" and set
predict_base64_path from "/api/predict/base64" to "/api/predict" so routes are
consistent with the router.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (4)
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (1)

39-41: Indentation issues from earlier review are fixed — thanks

The misindented comments noted previously are now aligned with surrounding code blocks.

Also applies to: 66-68

services/painting-surface-data-simulator-service/app/services/model_client.py (1)

100-101: Avoid bare except in health_check to prevent swallowing critical errors

Catching all exceptions hides unexpected failures. Catch httpx-specific exceptions instead.

-        except:
-            return False
+        except (httpx.HTTPError, httpx.ConnectError, httpx.TimeoutException):
+            return False
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)

30-33: Do not print connection string fragments — secrets exposure risk

Printing even partial connection strings can leak credentials. Replace with a generic message.

         except ClientAuthenticationError as e:
             print(f"❌ Azure Storage 인증 실패: {e}")
-            print(f"   연결 문자열을 확인해주세요: {self.connection_string[:50]}...")
+            print("   연결 문자열을 확인해주세요.")
             raise
services/painting-surface-data-simulator-service/requirements.txt (1)

15-16: httpx dependency already specified - remove duplicate.

The previous review comment correctly identified that httpx is defined twice in this file. Line 16 has httpx==0.25.2 (which appears to be missing from the current code but was in the previous version), and Line 22 has httpx>=0.24.0. This duplication should be resolved.

🧹 Nitpick comments (10)
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (1)

4-4: Remove unused import to satisfy linter

datetime is imported but never used.

-from datetime import datetime
services/painting-surface-data-simulator-service/app/services/model_client.py (3)

5-5: Remove unused import

datetime is not used in this module.

-from datetime import datetime

20-21: Drop unnecessary f-strings without placeholders

These f-strings have no interpolations; remove the f prefix.

-            print(f"❌ Painting Surface 서비스 URL을 찾을 수 없습니다.")
+            print("❌ Painting Surface 서비스 URL을 찾을 수 없습니다.")
@@
-        print(f"🎨 도장 표면 이미지 예측 요청...")
+        print("🎨 도장 표면 이미지 예측 요청...")
@@
-            print(f"   ✅ 결함 없음 - 정상 상태")
+            print("   ✅ 결함 없음 - 정상 상태")

Also applies to: 29-29, 164-164


76-80: Simplify conditional assignment

Use a ternary operator for brevity.

-        if defect_count > 0:
-            final_status = "anomaly"
-        else:
-            final_status = "normal"
+        final_status = "anomaly" if defect_count > 0 else "normal"
services/painting-surface-data-simulator-service/app/services/azure_storage.py (2)

1-5: Remove unused imports (lint: F401)

Drop Any, asyncio, and AzureError which are not used.

-from typing import List, Dict, Any, Optional
-import asyncio
+from typing import List, Dict, Optional
 from azure.storage.blob.aio import BlobServiceClient
-from azure.core.exceptions import AzureError, ClientAuthenticationError
+from azure.core.exceptions import ClientAuthenticationError

136-144: Batching should wrap to maintain consistent batch_size across cycles

Current slicing truncates at list end, yielding fewer items than batch_size. Use modular indexing to wrap.

-            batch_size = min(settings.batch_size, len(image_files))
-            start_idx = self.image_index % len(image_files)
-            end_idx = min(start_idx + batch_size, len(image_files))
-            
-            batch_files = image_files[start_idx:end_idx]
-            self.image_index = (self.image_index + batch_size) % len(image_files)
+            batch_size = min(settings.batch_size, len(image_files))
+            start_idx = self.image_index % len(image_files)
+            batch_files = [
+                image_files[(start_idx + i) % len(image_files)]
+                for i in range(batch_size)
+            ]
+            self.image_index = (self.image_index + batch_size) % len(image_files)
services/painting-surface-data-simulator-service/requirements.txt (4)

21-22: Consider pinning httpx version for production consistency.

While the flexible version constraint httpx>=0.24.0 allows for updates, pinning to a specific version (e.g., httpx==0.25.2) would ensure reproducible builds and avoid potential compatibility issues in production deployments.

-# HTTP 테스트 클라이언트
-httpx>=0.24.0
+# HTTP 테스트 클라이언트
+httpx==0.25.2

16-19: Consider consistent versioning strategy across test dependencies.

The testing dependencies mix pinned versions (pytest==7.4.3, pytest-asyncio==0.21.1) with flexible constraints (pytest-cov>=4.1.0, pytest-mock>=3.11.0). For better reproducibility, consider using consistent versioning strategy.


34-34: Missing newline at end of file.

The file should end with a newline character for better POSIX compliance and to avoid potential issues with some tools.

 # 개발 도구
 pre-commit>=3.3.0
+

21-33: Consider using English comments for consistency.

The comments are written in Korean while the rest of the codebase appears to use English. Consider using English comments for better international collaboration and consistency.

-# HTTP 테스트 클라이언트
+# HTTP test client
 httpx>=0.24.0

-# 코드 품질 도구
+# Code quality tools
 black>=23.0.0
 flake8>=6.0.0
 mypy>=1.5.0

-# 테스트 커버리지
+# Test coverage
 coverage>=7.2.0

-# 개발 도구
+# Development tools
 pre-commit>=3.3.0
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13796e3 and 9606a6c.

📒 Files selected for processing (4)
  • services/painting-surface-data-simulator-service/app/services/azure_storage.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/model_client.py (1 hunks)
  • services/painting-surface-data-simulator-service/requirements.txt (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py (5)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (6)
  • SimulatorScheduler (10-140)
  • stop (51-60)
  • start (15-49)
  • get_status (132-140)
  • _simulate_data_collection (76-130)
  • _initial_health_check (62-74)
services/painting-surface-data-simulator-service/tests/conftest.py (3)
  • mock_azure_storage (56-67)
  • mock_model_client (71-80)
  • mock_settings (36-52)
services/painting-surface-data-simulator-service/app/services/model_client.py (2)
  • health_check (89-101)
  • predict_painting_surface_data (17-62)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (3)
  • connect (17-36)
  • disconnect (53-56)
  • simulate_painting_surface_data (126-155)
services/painting-surface-data-simulator-service/app/utils/logger.py (3)
  • log_normal_processing (36-39)
  • log_anomaly (15-34)
  • log_error (41-55)
services/painting-surface-data-simulator-service/app/services/model_client.py (3)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • model_service_url (31-33)
services/painting-surface-data-simulator-service/app/main.py (1)
  • health_check (87-89)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)
  • read_image_data (88-108)
🪛 Ruff (0.12.2)
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py

4-4: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)


20-20: Local variable loop is assigned to but never used

Remove assignment to unused variable loop

(F841)

services/painting-surface-data-simulator-service/app/services/azure_storage.py

1-1: typing.Any imported but unused

Remove unused import: typing.Any

(F401)


2-2: asyncio imported but unused

Remove unused import: asyncio

(F401)


4-4: azure.core.exceptions.AzureError imported but unused

Remove unused import: azure.core.exceptions.AzureError

(F401)

services/painting-surface-data-simulator-service/app/services/model_client.py

5-5: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)


20-20: f-string without any placeholders

Remove extraneous f prefix

(F541)


29-29: f-string without any placeholders

Remove extraneous f prefix

(F541)


76-79: Use ternary operator final_status = "anomaly" if defect_count > 0 else "normal" instead of if-else-block

Replace if-else-block with final_status = "anomaly" if defect_count > 0 else "normal"

(SIM108)


100-100: Do not use bare except

(E722)


164-164: f-string without any placeholders

Remove extraneous f prefix

(F541)

🔇 Additional comments (1)
services/painting-surface-data-simulator-service/requirements.txt (1)

1-33: LGTM! Well-organized dependency structure.

The requirements file is well-organized with clear categorization of dependencies. The version pinning strategy for core dependencies ensures stability while allowing flexibility for development tools.

Comment on lines +154 to +163
predictions = result.get('predictions', [])
if predictions:
print(f" ⚠️ 결함 탐지됨: {len(predictions)}개")
for i, pred in enumerate(predictions, 1):
print(f" 결함 {i}:")
print(f" 🏷️ 종류: {pred.get('class_name', 'N/A')}")
print(f" 📍 위치: {pred.get('bbox', 'N/A')}")
print(f" 📏 크기: {pred.get('area', 'N/A')} 픽셀²")
print(f" 🎯 신뢰도: {pred.get('confidence', 'N/A'):.3f}")
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Formatting bug: guard non-numeric confidence before using .3f

If confidence is missing or non-numeric, f"{...:.3f}" will raise a TypeError. Guard it.

-        if predictions:
+        if predictions:
             print(f"   ⚠️  결함 탐지됨: {len(predictions)}개")
             for i, pred in enumerate(predictions, 1):
                 print(f"      결함 {i}:")
                 print(f"        🏷️  종류: {pred.get('class_name', 'N/A')}")
                 print(f"        📍 위치: {pred.get('bbox', 'N/A')}")
                 print(f"        📏 크기: {pred.get('area', 'N/A')} 픽셀²")
-                print(f"        🎯 신뢰도: {pred.get('confidence', 'N/A'):.3f}")
+                confidence = pred.get('confidence')
+                if isinstance(confidence, (int, float)):
+                    print(f"        🎯 신뢰도: {confidence:.3f}")
+                else:
+                    print(f"        🎯 신뢰도: {confidence if confidence is not None else 'N/A'}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
predictions = result.get('predictions', [])
if predictions:
print(f" ⚠️ 결함 탐지됨: {len(predictions)}개")
for i, pred in enumerate(predictions, 1):
print(f" 결함 {i}:")
print(f" 🏷️ 종류: {pred.get('class_name', 'N/A')}")
print(f" 📍 위치: {pred.get('bbox', 'N/A')}")
print(f" 📏 크기: {pred.get('area', 'N/A')} 픽셀²")
print(f" 🎯 신뢰도: {pred.get('confidence', 'N/A'):.3f}")
else:
predictions = result.get('predictions', [])
if predictions:
print(f" ⚠️ 결함 탐지됨: {len(predictions)}개")
for i, pred in enumerate(predictions, 1):
print(f" 결함 {i}:")
print(f" 🏷️ 종류: {pred.get('class_name', 'N/A')}")
print(f" 📍 위치: {pred.get('bbox', 'N/A')}")
print(f" 📏 크기: {pred.get('area', 'N/A')} 픽셀²")
confidence = pred.get('confidence')
if isinstance(confidence, (int, float)):
print(f" 🎯 신뢰도: {confidence:.3f}")
else:
print(f" 🎯 신뢰도: {confidence if confidence is not None else 'N/A'}")
else:
🤖 Prompt for AI Agents
In services/painting-surface-data-simulator-service/app/services/model_client.py
around lines 154 to 163, the code formats pred.get('confidence') with
"{...:.3f}" which will raise a TypeError if confidence is missing or
non-numeric; update the printing to guard the value first (e.g., retrieve
confidence = pred.get('confidence'), check isinstance(confidence, (int, float))
or attempt a safe float conversion in a try/except) and only apply the ".3f"
format when numeric, otherwise print a fallback like "N/A" (or the raw value) to
avoid exceptions.

Comment on lines +19 to +24
try:
loop = asyncio.get_running_loop()
asyncio.create_task(self.scheduler.stop())
except RuntimeError:
# 이벤트 루프가 실행 중이 아니면 무시
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Ensure scheduler.stop() is actually executed during teardown

Currently, a task is created but never awaited, and exceptions swallow the stop call. This can leak tasks and make tests flaky. Use the running loop to schedule the task; if no loop is running, run the coroutine to completion.

-            try:
-                loop = asyncio.get_running_loop()
-                asyncio.create_task(self.scheduler.stop())
-            except RuntimeError:
-                # 이벤트 루프가 실행 중이 아니면 무시
-                pass
+            try:
+                loop = asyncio.get_running_loop()
+                loop.create_task(self.scheduler.stop())
+            except RuntimeError:
+                # 이벤트 루프가 실행 중이 아니면 동기적으로 실행
+                asyncio.run(self.scheduler.stop())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try:
loop = asyncio.get_running_loop()
asyncio.create_task(self.scheduler.stop())
except RuntimeError:
# 이벤트 루프가 실행 중이 아니면 무시
pass
try:
loop = asyncio.get_running_loop()
loop.create_task(self.scheduler.stop())
except RuntimeError:
# 이벤트 루프가 실행 중이 아니면 동기적으로 실행
asyncio.run(self.scheduler.stop())
🧰 Tools
🪛 Ruff (0.12.2)

20-20: Local variable loop is assigned to but never used

Remove assignment to unused variable loop

(F841)

🤖 Prompt for AI Agents
In
services/painting-surface-data-simulator-service/tests/test_scheduler_service.py
around lines 19-24, the teardown currently creates a task for
self.scheduler.stop() without ensuring it actually runs or is awaited; change
the logic so that if an event loop is running you schedule and await the stop
coroutine (e.g., create the task and await it from async teardown), and if no
loop is running call asyncio.run(self.scheduler.stop()) to run the coroutine to
completion; ensure the stop coroutine is awaited in all code paths to avoid
leaking tasks and flakiness.

@leewonwook leewonwook changed the title Feat/34/painting surface defect simulator: 도장표면결함 시뮬레이터 기능 구현 Feat/34/painting-surface-defect-simulator: 도장표면결함 시뮬레이터 기능 구현 Aug 13, 2025
@leewonwook leewonwook changed the title Feat/34/painting-surface-defect-simulator: 도장표면결함 시뮬레이터 기능 구현 Feat/34/painting-surface-defect-simulator-service: 도장표면결함 시뮬레이터 기능 구현 Aug 13, 2025
@leewonwook leewonwook changed the title Feat/34/painting-surface-defect-simulator-service: 도장표면결함 시뮬레이터 기능 구현 Feat/34/painting-surface-data-simulator-service: 도장표면결함 시뮬레이터 기능 구현 Aug 13, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
services/painting-surface-data-simulator-service/app/config/settings.py (1)

8-9: Add fail-fast validation for required AZURE_CONNECTION_STRING.

Make the error explicit and user-friendly when the env var is missing or blank.

 class Settings(BaseSettings):
     # Azure Storage 설정
     azure_connection_string: str
     azure_container_name: str = "simulator-data"
+
+    @field_validator("azure_connection_string")
+    @classmethod
+    def validate_azure_connection_string(cls, v: str) -> str:
+        if not v or not v.strip():
+            raise ValueError(
+                "AZURE_CONNECTION_STRING 환경 변수가 설정되지 않았습니다. "
+                ".env 파일 또는 실행 환경에서 Azure Storage 연결 문자열을 설정해주세요."
+            )
+        return v
services/painting-surface-data-simulator-service/tests/test_settings.py (1)

9-13: Fix test: set required env var before instantiating Settings.

Settings requires AZURE_CONNECTION_STRING; without it, instantiation will fail.

     def test_settings_default_values(self):
         """기본값 테스트"""
-        # 환경 변수가 설정되어 있으므로 기본값 확인 테스트
-        settings = Settings()
+        # 필수 환경 변수 설정 후 기본값 확인 테스트
+        os.environ['AZURE_CONNECTION_STRING'] = 'test_connection_string'
+        try:
+            settings = Settings()
+        finally:
+            if 'AZURE_CONNECTION_STRING' in os.environ:
+                del os.environ['AZURE_CONNECTION_STRING']
🧹 Nitpick comments (7)
services/painting-surface-data-simulator-service/app/config/settings.py (1)

35-38: Use SettingsConfigDict for model_config (pydantic v2 idiomatic).

Be explicit with SettingsConfigDict to align with pydantic-settings v2 best practices.

-    model_config = {
-        "env_file": ".env",
-        "env_file_encoding": "utf-8"
-    }
+    model_config: SettingsConfigDict = SettingsConfigDict(
+        env_file=".env",
+        env_file_encoding="utf-8",
+    )
services/painting-surface-data-simulator-service/tests/test_settings.py (2)

1-1: Remove unused import (pytest).

Clean up unused import flagged by Ruff.

-import pytest

3-3: Heads-up: importing Settings triggers settings module import-time side effects.

If app/config/settings.py keeps a module-level instance (settings = Settings()), this import will fail unless AZURE_CONNECTION_STRING is set before the test module import. Prefer the lazy get_settings() approach or ensure env vars are set globally (e.g., in conftest with an autouse fixture).

services/painting-surface-data-simulator-service/tests/conftest.py (4)

4-5: Remove unused imports (sys, Path).

Clean up unused imports flagged by Ruff.

 import asyncio
 import os
-import sys
-from pathlib import Path

29-35: Consider using pytest-asyncio’s event loop fixture (session scope).

Custom function-scoped event loop can conflict with pytest-asyncio versions that expect a session-scoped loop. Prefer the plugin’s fixture or align scope with it.

Option:

-@pytest.fixture
-def event_loop():
-    """비동기 테스트를 위한 이벤트 루프 fixture"""
-    loop = asyncio.new_event_loop()
-    yield loop
-    loop.close()
+import pytest_asyncio
+
+@pytest_asyncio.fixture(scope="session")
+def event_loop():
+    """비동기 테스트를 위한 이벤트 루프 fixture (session scope)"""
+    loop = asyncio.new_event_loop()
+    yield loop
+    loop.close()

Verify pytest-asyncio version compatibility in your environment before applying.


65-68: Align mock simulate_painting_surface_data() shape with production code.

The real service returns images, total_images, batch_size, current_batch. The mock adds metadata and omits totals, which can mask mismatches.

     mock_storage.simulate_painting_surface_data = AsyncMock(return_value={
-        "images": ["test1.jpg", "test2.jpg"],
-        "metadata": {"source": "test"}
+        "images": ["test1.jpg", "test2.jpg"],
+        "total_images": 2,
+        "batch_size": 2,
+        "current_batch": ["test1.jpg", "test2.jpg"]
     })

76-81: Align mock model_client return with service contract.

Model client predict_painting_surface_data() in app/services/model_client.py returns a structure with images and combined. The mock returns status/defect_count/total_count, which may hide integration issues.

-    mock_client.predict_painting_surface_data = AsyncMock(return_value={
-        "status": "normal",
-        "defect_count": 0,
-        "total_count": 2
-    })
+    mock_client.predict_painting_surface_data = AsyncMock(return_value={
+        "images": [
+            {"file": "test1.jpg", "result": {"defect": False}},
+            {"file": "test2.jpg", "result": {"defect": False}},
+        ],
+        "combined": {"defect_count": 0, "total_count": 2}
+    })
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between baf8d23 and 57e5e31.

📒 Files selected for processing (4)
  • services/painting-surface-data-simulator-service/app/config/settings.py (1 hunks)
  • services/painting-surface-data-simulator-service/requirements.txt (1 hunks)
  • services/painting-surface-data-simulator-service/tests/conftest.py (1 hunks)
  • services/painting-surface-data-simulator-service/tests/test_settings.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • services/painting-surface-data-simulator-service/requirements.txt
🧰 Additional context used
🧬 Code Graph Analysis (2)
services/painting-surface-data-simulator-service/tests/test_settings.py (1)
services/painting-surface-data-simulator-service/app/config/settings.py (2)
  • Settings (6-38)
  • model_service_url (31-33)
services/painting-surface-data-simulator-service/tests/conftest.py (3)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • model_service_url (31-33)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (2)
  • read_image_data (88-108)
  • simulate_painting_surface_data (126-155)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)
  • predict_painting_surface_data (17-62)
🪛 Ruff (0.12.2)
services/painting-surface-data-simulator-service/tests/test_settings.py

1-1: pytest imported but unused

Remove unused import: pytest

(F401)

services/painting-surface-data-simulator-service/app/config/settings.py

1-1: os imported but unused

Remove unused import: os

(F401)


2-2: typing.Dict imported but unused

Remove unused import: typing.Dict

(F401)

services/painting-surface-data-simulator-service/tests/conftest.py

4-4: sys imported but unused

Remove unused import: sys

(F401)


5-5: pathlib.Path imported but unused

Remove unused import: pathlib.Path

(F401)

Comment on lines +1 to +4
import os
from typing import Dict
from pydantic_settings import BaseSettings

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consolidate imports: drop unused and add required ones (field_validator, SettingsConfigDict, lru_cache).

Remove unused imports flagged by Ruff and add the imports needed for validation and lazy settings loading.

-import os
-from typing import Dict
-from pydantic_settings import BaseSettings
+from functools import lru_cache
+from pydantic import field_validator
+from pydantic_settings import BaseSettings, SettingsConfigDict
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import os
from typing import Dict
from pydantic_settings import BaseSettings
from functools import lru_cache
from pydantic import field_validator
from pydantic_settings import BaseSettings, SettingsConfigDict
🧰 Tools
🪛 Ruff (0.12.2)

1-1: os imported but unused

Remove unused import: os

(F401)


2-2: typing.Dict imported but unused

Remove unused import: typing.Dict

(F401)

🤖 Prompt for AI Agents
In services/painting-surface-data-simulator-service/app/config/settings.py
around lines 1-4, remove the unused imports flagged by Ruff (e.g., typing.Dict
if it's not used) and add the missing imports required for validation and lazy
loading: import field_validator and SettingsConfigDict from pydantic (or
pydantic.v2 appropriate module) and import lru_cache from functools; ensure
imports are ordered and minimal so the module uses BaseSettings, field_validator
and SettingsConfigDict for pydantic settings validation and lru_cache for a
cached settings factory.

}


settings = Settings()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Avoid module-level instantiation; switch to lazy, cached settings loader.

Instantiating Settings at import time makes imports fail when required env vars are missing (e.g., in tests without a .env). Prefer a lazy loader with caching.

-settings = Settings()
+@lru_cache(maxsize=1)
+def get_settings() -> Settings:
+    """Lazy-load and cache Settings; avoids import-time side effects."""
+    return Settings()

Follow-up (outside this file): update call sites to use get_settings().

Example:

# before
from app.config.settings import settings

# after
from app.config.settings import get_settings
settings = get_settings()

Please verify that all usages across the service are updated accordingly (main.py, services/*, utils/logger.py). If you prefer to keep the module-level variable for now, ensure tests set AZURE_CONNECTION_STRING before importing settings.py to avoid import-time ValidationError.


🏁 Script executed:

#!/bin/bash
# Locate all direct imports of the module-level `settings`
rg -n "from .*settings import settings"

# Find all property/method usages of the `settings` instance
rg -n "settings\."

Length of output: 78259


Refactor module-level Settings instantiation to a lazy, cached loader

In services/painting-surface-data-simulator-service/app/config/settings.py replace:

- settings = Settings()
+ @lru_cache(maxsize=1)
+ def get_settings() -> Settings:
+     """Lazy-load and cache Settings to avoid import-time side effects."""
+     return Settings()

Then update every call site that currently does:

from app.config.settings import settings

to:

from app.config.settings import get_settings
settings = get_settings()

This includes, but isn’t limited to:

  • app/main.py
  • All modules under app/services/
  • app/utils/logger.py
  • All tests in services/painting-surface-data-simulator-service/tests/

Until this refactor is in place, be sure your test setup injects required environment variables (e.g. AZURE_CONNECTION_STRING) before importing settings.py to avoid import-time ValidationError.

🤖 Prompt for AI Agents
In services/painting-surface-data-simulator-service/app/config/settings.py
around line 41 replace the module-level instantiation "settings = Settings()"
with a lazy, cached loader function (e.g., define a get_settings() that
constructs and caches a Settings instance on first call) so settings are not
created at import time; then update every call site that currently does "from
app.config.settings import settings" to "from app.config.settings import
get_settings" and call "settings = get_settings()" at runtime (apply to
app/main.py, all modules under app/services/, app/utils/logger.py, and all tests
under services/.../tests/); until all call sites are updated, ensure tests set
required environment variables (like AZURE_CONNECTION_STRING) before importing
settings.py to avoid import-time ValidationError.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (5)
services/painting-surface-data-simulator-service/app/main.py (1)

3-3: Prefer lazy, cached settings to avoid import-time failures

Importing a module-level settings instance can raise at import time if required env vars are missing. Switch to a get_settings() factory and call it in this module.

After implementing get_settings() in settings.py:

-from app.config.settings import settings
+from app.config.settings import get_settings
+settings = get_settings()
services/painting-surface-data-simulator-service/app/config/settings.py (2)

49-50: Avoid module-level instantiation; provide a cached factory

Prevents import-time failures and improves testability.

-settings = Settings()
+@lru_cache(maxsize=1)
+def get_settings() -> Settings:
+    """Lazy-load and cache Settings; avoids import-time side effects."""
+    return Settings()

Follow-up: update call sites to import get_settings and do settings = get_settings().


8-8: Validate required env var azure_connection_string

Fail fast with a clear message if missing or blank.

 class Settings(BaseSettings):
     # Azure Storage 설정
     azure_connection_string: str
+
+    @field_validator('azure_connection_string')
+    @classmethod
+    def validate_azure_connection_string(cls, v: str) -> str:
+        if not v or not v.strip():
+            raise ValueError(
+                "AZURE_CONNECTION_STRING 환경 변수가 설정되지 않았습니다. "
+                ".env 파일 또는 배포 환경에서 Azure Storage 연결 문자열을 설정해주세요."
+            )
+        return v
services/painting-surface-data-simulator-service/app/services/model_client.py (2)

98-104: Avoid bare except; handle httpx exceptions explicitly

Use explicit exceptions so real issues aren’t silenced.

-        except:
-            return False
+        except (httpx.HTTPError, httpx.ConnectError, httpx.TimeoutException):
+            return False

165-170: Guard confidence before using .3f to avoid TypeError

Formatting non-numeric values with :.3f will raise. Guard it.

-                print(f"        🎯 신뢰도: {pred.get('confidence', 'N/A'):.3f}")
+                confidence = pred.get('confidence')
+                if isinstance(confidence, (int, float)):
+                    print(f"        🎯 신뢰도: {confidence:.3f}")
+                else:
+                    print(f"        🎯 신뢰도: {confidence if confidence is not None else 'N/A'}")
🧹 Nitpick comments (9)
services/painting-surface-data-simulator-service/app/main.py (2)

8-8: Remove unused import painting_surface_model_client

Imported but never used. Clean it up to satisfy Ruff (F401) and avoid confusion.

-from app.services.model_client import painting_surface_model_client

44-45: Drop unnecessary f-string prefix

These lines contain no placeholders; remove the f prefix to satisfy Ruff (F541).

-    print(f"🔧 스케줄러 간격: {settings.scheduler_interval_minutes}분")
-    print(f"🎯 대상 서비스: 도장 표면 결함탐지 모델")
+    print(f"🔧 스케줄러 간격: {settings.scheduler_interval_minutes}분")
+    print("🎯 대상 서비스: 도장 표면 결함탐지 모델")

Note: Only the second line needed the change; left the first intact since it has a placeholder.

services/painting-surface-data-simulator-service/app/services/scheduler_service.py (2)

43-46: Drop unnecessary f-strings without placeholders

Ruff (F541): remove f prefix where no formatting occurs.

-            print(f"📊 대상 서비스: 도장 표면 결함탐지 모델")
+            print("📊 대상 서비스: 도장 표면 결함탐지 모델")
@@
-        print(f"📈 활성 서비스: 1/1")
+        print("📈 활성 서비스: 1/1")

Also applies to: 73-75


28-36: Prevent overlapping job runs (coalesce and max_instances)

If a run takes longer than the interval, multiple instances may overlap depending on scheduler defaults. Make this explicit to avoid concurrent executions of the job.

             self.scheduler.add_job(
                 func=self._simulate_data_collection,
                 trigger=IntervalTrigger(
                     minutes=settings.scheduler_interval_minutes),
                 id='data_simulation',
                 name='Data Collection Simulation',
-                replace_existing=True
+                replace_existing=True,
+                max_instances=1,
+                coalesce=True,
+                misfire_grace_time=60
             )
services/painting-surface-data-simulator-service/app/config/settings.py (2)

1-4: Remove unused imports and add required ones (validator, config, cache)

os and Dict are unused. You’ll need field_validator, SettingsConfigDict, and lru_cache if adopting validation and lazy loading.

-import os
-from typing import Dict
-from pydantic_settings import BaseSettings
+from functools import lru_cache
+from pydantic import field_validator
+from pydantic_settings import BaseSettings, SettingsConfigDict

43-46: Use SettingsConfigDict and consider ignoring extra envs

Minor consistency/pydantic v2 idiom improvement.

-    model_config = {
-        "env_file": ".env",
-        "env_file_encoding": "utf-8"
-    }
+    model_config = SettingsConfigDict(
+        env_file=".env",
+        env_file_encoding="utf-8",
+        extra="ignore",
+    )
services/painting-surface-data-simulator-service/app/services/model_client.py (3)

5-5: Remove unused import datetime

Not used. Satisfies Ruff (F401).

-from datetime import datetime

21-22: Drop unnecessary f-string prefixes

Strings have no placeholders. Remove f to satisfy Ruff (F541).

-            print(f"❌ Backend 서비스 URL을 찾을 수 없습니다.")
+            print("❌ Backend 서비스 URL을 찾을 수 없습니다.")
@@
-        print(f"🎨 도장 표면 이미지 예측 요청...")
+        print("🎨 도장 표면 이미지 예측 요청...")
@@
-            print(f"   ✅ 결함 없음 - 정상 상태")
+            print("   ✅ 결함 없음 - 정상 상태")

Also applies to: 30-31, 171-171


4-4: Align with lazy settings (if adopted)

If you switch to get_settings(), update usages here to avoid import-time config capture.

-from app.config.settings import settings
+from app.config.settings import get_settings
+settings = get_settings()
@@
-        self.backend_url = settings.backend_service_url
+        self.backend_url = settings.backend_service_url

Note: If backend URL can change at runtime, consider resolving from settings per request instead of caching in init.

Also applies to: 16-16

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 29d0951 and 1213f00.

📒 Files selected for processing (4)
  • services/painting-surface-data-simulator-service/app/config/settings.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/main.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/model_client.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/scheduler_service.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
services/painting-surface-data-simulator-service/app/config/settings.py (3)
services/press-fault-data-simulator-service/app/config/settings.py (1)
  • Settings (7-47)
services/press-fault-data-simulator-service/app/services/scheduler_service.py (1)
  • SchedulerService (12-208)
services/press-fault-data-simulator-service/tests/utils/test_logger.py (1)
  • fake_settings (50-74)
services/painting-surface-data-simulator-service/app/services/model_client.py (2)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • backend_service_url (39-41)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)
  • read_image_data (88-108)
services/painting-surface-data-simulator-service/app/main.py (4)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • backend_service_url (39-41)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)
  • connect (17-36)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (2)
  • stop (51-60)
  • get_status (132-140)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)
  • health_check (90-103)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (4)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (2)
  • disconnect (53-56)
  • simulate_painting_surface_data (126-155)
services/painting-surface-data-simulator-service/app/main.py (1)
  • health_check (87-89)
services/painting-surface-data-simulator-service/app/services/model_client.py (2)
  • health_check (90-103)
  • predict_painting_surface_data (18-63)
services/painting-surface-data-simulator-service/app/utils/logger.py (3)
  • log_anomaly (15-34)
  • log_normal_processing (36-39)
  • log_error (41-55)
🪛 Ruff (0.12.2)
services/painting-surface-data-simulator-service/app/config/settings.py

1-1: os imported but unused

Remove unused import: os

(F401)


2-2: typing.Dict imported but unused

Remove unused import: typing.Dict

(F401)

services/painting-surface-data-simulator-service/app/services/model_client.py

5-5: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)


21-21: f-string without any placeholders

Remove extraneous f prefix

(F541)


30-30: f-string without any placeholders

Remove extraneous f prefix

(F541)


77-80: Use ternary operator final_status = "anomaly" if defect_count > 0 else "normal" instead of if-else-block

Replace if-else-block with final_status = "anomaly" if defect_count > 0 else "normal"

(SIM108)


102-102: Do not use bare except

(E722)


171-171: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/app/main.py

8-8: app.services.model_client.painting_surface_model_client imported but unused

Remove unused import: app.services.model_client.painting_surface_model_client

(F401)


44-44: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/app/services/scheduler_service.py

44-44: f-string without any placeholders

Remove extraneous f prefix

(F541)


73-73: f-string without any placeholders

Remove extraneous f prefix

(F541)

🔇 Additional comments (3)
services/painting-surface-data-simulator-service/app/main.py (1)

69-90: Endpoints and lifecycle look solid

Root and health endpoints are straightforward; lifespan handles startup diagnostics and graceful shutdown of the scheduler. LGTM.

services/painting-surface-data-simulator-service/app/services/scheduler_service.py (1)

51-61: Shutdown sequence looks correct

Gracefully shuts down APScheduler and disconnects Azure storage; state flag updated. Good.

services/painting-surface-data-simulator-service/app/services/model_client.py (1)

115-153: HTTP client usage and retries look good

Multipart upload with retries and exponential backoff is appropriate; error handling is clear. LGTM.

# 로그 설정
log_directory: str = "logs"
log_filename: str = "painting_defect_detections.json"
error_log_filename: str = "painting_defect_detections.json"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Separate error log filename from normal log filename

Both are "painting_defect_detections.json", which mixes normal and error logs in one file. Use a distinct error filename.

-    error_log_filename: str = "painting_defect_detections.json"
+    error_log_filename: str = "painting_defect_errors.json"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
error_log_filename: str = "painting_defect_detections.json"
error_log_filename: str = "painting_defect_errors.json"
🤖 Prompt for AI Agents
In services/painting-surface-data-simulator-service/app/config/settings.py
around line 27, the error_log_filename is set to the same value as the normal
log file ("painting_defect_detections.json"), which mixes normal and error logs;
change error_log_filename to a distinct filename (e.g.,
"painting_defect_errors.json" or similar), update any references or tests that
read/write the error log to use the new name, and ensure logging configuration
writes error-level entries to this separate file.

Comment on lines +81 to +82
"connection_status": "connected" if hasattr(azure_storage, 'client') and azure_storage.client else "disconnected"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Connection status may be stale after disconnect

/ reports "connected" if azure_storage.client is truthy. In azure_storage.disconnect(), the client is closed but not set to None, so status can incorrectly remain "connected" after shutdown.

Proposed fix in azure_storage (outside this file):

 async def disconnect(self):
     """연결 종료"""
     if self.client:
         await self.client.close()
+        self.client = None

Alternatively, expose an is_connected property on the service and use it here.


🏁 Script executed:

#!/bin/bash
# Locate references to azure_storage in the repo
echo "== Files importing azure_storage =="
rg -l "azure_storage" -g "*.py" || true

# Show usage in main.py around the reported lines
echo -e "\n== services/.../main.py around connection_status =="
sed -n '1,150p' services/painting-surface-data-simulator-service/app/main.py | nl | sed -n '70,100p'

# Find all disconnect() definitions across Python files
echo -e "\n== All disconnect() methods =="
rg -n "def disconnect" -C3 -g "*.py" || true

# If azure_storage disconnect exists, show its implementation
echo -e "\n== azure_storage.disconnect implementation =="
# First find the file defining AzureStorage
rg -l "class .*AzureStorage" -g "*.py" | head -n1 | xargs sed -n '1,200p'

Length of output: 12216


🏁 Script executed:

#!/bin/bash
echo "== main.py (75-90) =="
sed -n '75,90p' services/painting-surface-data-simulator-service/app/main.py

echo -e "\n== painting-process azure_storage.py (1-80) =="
sed -n '1,80p' services/painting-process-data-simulator-service/app/services/azure_storage.py

Length of output: 3870


🏁 Script executed:

#!/bin/bash
# Show the full implementation of the AzureStorageService in the painting-surface service
sed -n '1,200p' services/painting-surface-data-simulator-service/app/services/azure_storage.py

Length of output: 5579


Fix stale connection_status after disconnect

In main.py, the status is derived from azure_storage.client being truthy. However, in
services/painting-surface-data-simulator-service/app/services/azure_storage.py,
disconnect() closes the client but never clears it, so connection_status will incorrectly remain "connected" after shutdown.

Please update disconnect() to reset the client reference:

File: services/painting-surface-data-simulator-service/app/services/azure_storage.py

 async def disconnect(self):
     """연결 종료"""
     if self.client:
         await self.client.close()
+        self.client = None

Alternatively, you could add an is_connected property on AzureStorageService and use that in main.py instead of directly inspecting client.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"connection_status": "connected" if hasattr(azure_storage, 'client') and azure_storage.client else "disconnected"
}
async def disconnect(self):
"""연결 종료"""
if self.client:
await self.client.close()
self.client = None
🤖 Prompt for AI Agents
In services/painting-surface-data-simulator-service/app/main.py around lines
81-82 the connection_status is derived from azure_storage.client which stays
truthy after shutdown because disconnect() in services/azure_storage.py closes
the client but does not clear the reference; open
services/painting-surface-data-simulator-service/app/services/azure_storage.py
and update disconnect() to both close/await close on the client and then set
self.client = None (or equivalent) so that main.py's truthy check reflects the
real state; alternatively add an is_connected property on AzureStorageService
that returns a boolean (e.g., bool(self.client) and not self.client.closed) and
switch main.py to use azure_storage.is_connected instead of inspecting client
directly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)

1-187: Refactor bare except: and placeholderless f-strings in model_client.py

The quick lint check shows one bare except: and several f-strings without placeholders in the painting-surface model client. These should be corrected to:

  • Narrow the except: to at least except Exception as e: (or a more specific exception).
  • Replace f-strings with no interpolations with plain string literals.

Please update the following spots in services/painting-surface-data-simulator-service/app/services/model_client.py:

  • Line 102:
    -        except:
    +        except Exception as e:
                 print(f"❌ Azure Storage에서 이미지 다운로드 실패 ({image_path}): {e}")
  • Line 21:
    -        if not self.backend_url:
    -            print(f"❌ Backend 서비스 URL을 찾을 수 없습니다.")
    +        if not self.backend_url:
    +            print("❌ Backend 서비스 URL을 찾을 수 없습니다.")
  • Line 30:
    -        print(f"🎨 도장 표면 이미지 예측 요청...")
    +        print("🎨 도장 표면 이미지 예측 요청...")
  • Line 171:
    -            print(f"   ✅ 결함 없음 - 정상 상태")
    +            print("   ✅ 결함 없음 - 정상 상태")
  • Line 177:
    -        print(f"   🔍 전체 응답 구조:")
    +        print("   🔍 전체 응답 구조:")
♻️ Duplicate comments (7)
services/painting-surface-data-simulator-service/app/config/settings.py (3)

24-27: Separate error log and normal log filenames.

Both point to the same file, which mixes concerns and complicates triage.

     # 로그 설정
     log_directory: str = "logs"
     log_filename: str = "painting_defect_detections.json"
-    error_log_filename: str = "painting_defect_detections.json"
+    error_log_filename: str = "painting_defect_errors.json"

6-13: Fail fast on missing AZURE_CONNECTION_STRING.

azure_connection_string lacks validation. Add a validator to provide a clear startup error when it’s empty or missing.

 class Settings(BaseSettings):
     # Azure Storage 설정
     azure_connection_string: str
     azure_container_name: str = "simulator-data"
+
+    @field_validator("azure_connection_string")
+    @classmethod
+    def _validate_azure_conn_str(cls, v: str) -> str:
+        if not v or not v.strip():
+            raise ValueError(
+                "AZURE_CONNECTION_STRING 환경 변수가 설정되지 않았습니다. "
+                ".env 파일에서 Azure Storage 연결 문자열을 설정해주세요."
+            )
+        return v

43-49: Use SettingsConfigDict and avoid import-time side effects with a cached factory.

  • Replace the raw dict with SettingsConfigDict (pydantic v2).
  • Swap the module-level singleton for a lazy, cached loader to prevent import-time failures when env isn’t set (especially in tests).
-    model_config = {
-        "env_file": ".env",
-        "env_file_encoding": "utf-8"
-    }
+    model_config = SettingsConfigDict(
+        env_file=".env",
+        env_file_encoding="utf-8",
+        extra="ignore",
+    )
 
-
-settings = Settings()
+@lru_cache(maxsize=1)
+def get_settings() -> Settings:
+    """Lazy-load and cache Settings; avoids import-time side effects."""
+    return Settings()

Run to find and update call sites importing the module-level settings:

#!/bin/bash
# Find direct imports of the singleton
rg -nP --type=py -C2 '\bfrom\s+app\.config\.settings\s+import\s+settings\b'

# Find attribute uses that may need local aliasing after switching to get_settings()
rg -nP --type=py -C1 '\bsettings\.[A-Z_a-z0-9]+\b' services/painting-surface-data-simulator-service/
services/painting-surface-data-simulator-service/app/services/azure_storage.py (1)

99-106: Batch wrap-around to always return full batches.

Current slicing returns fewer files near the list tail. If you want a true circular batch, wrap indices.

-            end_idx = min(start_idx + batch_size, len(image_files))
-            
-            batch_files = image_files[start_idx:end_idx]
-            self.image_index = (self.image_index + batch_size) % len(image_files)
+            batch_files = []
+            for i in range(batch_size):
+                idx = (start_idx + i) % len(image_files)
+                batch_files.append(image_files[idx])
+            self.image_index = (start_idx + batch_size) % len(image_files)
services/painting-surface-data-simulator-service/app/routers/test_connection_router.py (1)

4-10: Remove pytest artifacts and unused import from production router.

pytestmark belongs in tests, not in the FastAPI router. Also, settings is unused.

-from app.config.settings import settings
-import pytest
+# removed unused settings import
@@
-router = APIRouter()
-
-pytestmark = pytest.mark.asyncio
+router = APIRouter()
services/painting-surface-data-simulator-service/app/services/model_client.py (2)

98-103: Replace bare except with specific httpx exception handling

Catching everything hides real bugs and trips Ruff E722. Handle httpx failures explicitly and log the error.

-        try:
-            async with httpx.AsyncClient(timeout=self.timeout) as client:
-                response = await client.get(health_url)
-                return response.status_code == 200
-        except:
-            return False
+        try:
+            async with httpx.AsyncClient(timeout=self.timeout) as client:
+                response = await client.get(health_url)
+                return response.status_code == 200
+        except httpx.HTTPError as e:
+            print(f"❌ 헬스 체크 실패: {e}")
+            return False

161-170: Guard non-numeric confidence before formatting with .3f

If confidence is missing or non-numeric, this print raises TypeError. Guard it.

         if predictions:
             print(f"   ⚠️  결함 탐지됨: {len(predictions)}개")
             for i, pred in enumerate(predictions, 1):
                 print(f"      결함 {i}:")
                 print(f"        🏷️  종류: {pred.get('class_name', 'N/A')}")
                 print(f"        📍 위치: {pred.get('bbox', 'N/A')}")
                 print(f"        📏 크기: {pred.get('area', 'N/A')} 픽셀²")
-                print(f"        🎯 신뢰도: {pred.get('confidence', 'N/A'):.3f}")
+                confidence = pred.get('confidence')
+                if isinstance(confidence, (int, float)):
+                    print(f"        🎯 신뢰도: {confidence:.3f}")
+                else:
+                    print(f"        🎯 신뢰도: {confidence if confidence is not None else 'N/A'}")
🧹 Nitpick comments (19)
services/painting-surface-data-simulator-service/app/config/settings.py (1)

1-4: Trim unused imports and add required ones (field_validator, SettingsConfigDict, lru_cache).

Ruff flags os and typing.Dict as unused. Also, you’ll need field_validator, SettingsConfigDict, and lru_cache for validation and lazy settings loading.

-import os
-from typing import Dict
-from pydantic_settings import BaseSettings
+from functools import lru_cache
+from pydantic import field_validator
+from pydantic_settings import BaseSettings, SettingsConfigDict
services/painting-surface-data-simulator-service/app/services/azure_storage.py (3)

1-5: Remove unused imports (Any, asyncio, AzureError).

Streamline imports per Ruff hints.

-from typing import List, Dict, Any, Optional
-import asyncio
+from typing import List, Dict, Optional
 from azure.storage.blob.aio import BlobServiceClient
-from azure.core.exceptions import AzureError, ClientAuthenticationError
+from azure.core.exceptions import ClientAuthenticationError
 from app.config.settings import settings

18-47: Prefer service logger over print statements.

For consistency with the PR’s logging system and to support JSON/file logging, route messages through the app logger (e.g., system_log) instead of print(). This will also centralize log level control.

If you can confirm the logger path (e.g., from app.utils.logger import system_log), I can provide a concrete patch replacing prints with system_log.info()/warning()/error().


121-122: Consider lazy, DI-friendly instantiation for the global service.

Creating a global instance at import time ties this module to configuration state and complicates testing. Prefer a small factory (or wire it in main.py) and cache if needed.

-azure_storage = AzureStorageService()
+# Consider wiring in app startup for better lifecycle control:
+# from functools import lru_cache
+# @lru_cache(maxsize=1)
+# def get_azure_storage() -> AzureStorageService:
+#     return AzureStorageService()
services/painting-surface-data-simulator-service/Dockerfile (1)

7-9: Optional: slim base image and non-root.

You can reduce size and improve security hardening by moving to python:3.10-slim, installing build deps only when needed, and switching to a non-root user.

I can draft a multi-stage Dockerfile (builder + runtime) and add a non-root user if you’d like.

services/painting-surface-data-simulator-service/app/routers/test_connection_router.py (1)

31-47: Optional: use HTTPException and typed responses.

Right now, endpoints always return 200 with a "status" field. For clearer API semantics, raise HTTPException(status_code=503, ...) on failures and define ResponseModel via Pydantic for both endpoints.

If you want, I can provide response models and exceptions with minimal changes to tests.

services/painting-surface-data-simulator-service/app/main.py (2)

45-45: Drop unnecessary f-string.

Literal string has no placeholders.

-    print(f"🎯 대상 서비스: 도장 표면 결함탐지 모델")
+    print("🎯 대상 서비스: 도장 표면 결함탐지 모델")

8-8: Remove unused import.

painting_surface_model_client isn’t used in this module.

-from app.services.model_client import painting_surface_model_client
services/painting-surface-data-simulator-service/app/services/model_client.py (7)

5-5: Remove unused import

datetime is imported but not used in this module. Clean it up to satisfy Ruff F401.

-from datetime import datetime

21-21: Drop f-strings without placeholders

These f-strings have no {}. Remove the f prefix (Ruff F541).

-            print(f"❌ Backend 서비스 URL을 찾을 수 없습니다.")
+            print("❌ Backend 서비스 URL을 찾을 수 없습니다.")
-        print(f"🎨 도장 표면 이미지 예측 요청...")
+        print("🎨 도장 표면 이미지 예측 요청...")
-            print(f"   ✅ 결함 없음 - 정상 상태")
+            print("   ✅ 결함 없음 - 정상 상태")
-        print(f"   🔍 전체 응답 구조:")
+        print("   🔍 전체 응답 구조:")

Also applies to: 30-30, 171-171, 177-177


77-81: Simplify final_status assignment

Minor readability: replace if/else with a ternary (Ruff SIM108).

-        if defect_count > 0:
-            final_status = "anomaly"
-        else:
-            final_status = "normal"
+        final_status = "anomaly" if defect_count > 0 else "normal"

117-151: Reuse a single AsyncClient across retries to reduce overhead

Creating a new client each attempt adds connection setup cost. Reuse one client and precompute files.

-        for attempt in range(self.max_retries):
-            try:
-                # multipart/form-data로 이미지 파일 업로드
-                files = {
-                    'image': (image_file, image_data, 'image/jpeg')
-                }
-                
-                async with httpx.AsyncClient(timeout=self.timeout) as client:
-                    response = await client.post(backend_url, files=files)
+        files = {'image': (image_file, image_data, 'image/jpeg')}
+        async with httpx.AsyncClient(timeout=self.timeout) as client:
+            for attempt in range(self.max_retries):
+                try:
+                    response = await client.post(backend_url, files=files)
@@
-            if attempt < self.max_retries - 1:
-                await asyncio.sleep(2 ** attempt)  # 지수 백오프
+                if attempt < self.max_retries - 1:
+                    await asyncio.sleep(2 ** attempt)  # 지수 백오프

50-53: Avoid catching bare Exception in per-image loop unless you log details

You’re swallowing any unexpected error. If you keep this broad handler, at least include traceback for postmortem or narrow the exceptions.

-            except Exception as e:
-                print(f"❌ 이미지 {image_file} 예측 실패: {e}")
+            except httpx.HTTPError as e:
+                print(f"❌ 이미지 {image_file} 예측 실패: {e}")
+            except Exception as e:
+                # Unexpected error
+                print(f"❌ 이미지 {image_file} 예측 처리 중 예기치 않은 오류: {e}")
                 continue

105-114: Consider narrowing exception on Azure download

If read_image_data raises known exceptions, catch those specifically; otherwise keep and add more context (path, type).


1-187: Optional: migrate prints to a structured logger

Given you already have anomaly_logger elsewhere, consider standardizing logs here too for consistency and better observability. Not a blocker.

services/painting-surface-data-simulator-service/app/services/scheduler_service.py (4)

44-44: Drop f-strings without placeholders

No {} in these strings. Remove the f prefix (Ruff F541).

-            print(f"📊 대상 서비스: 도장 표면 결함탐지 모델")
+            print("📊 대상 서비스: 도장 표면 결함탐지 모델")
@@
-        print(f"📈 활성 서비스: 1/1")
+        print("📈 활성 서비스: 1/1")

Also applies to: 73-73


29-36: Prevent overlapping runs and handle misfires

Set max_instances=1 (explicit), enable coalesce, and add a modest misfire_grace_time to avoid piling up jobs if the loop stalls.

             self.scheduler.add_job(
                 func=self._simulate_data_collection,
                 trigger=IntervalTrigger(
                     minutes=settings.scheduler_interval_minutes),
                 id='data_simulation',
                 name='Data Collection Simulation',
-                replace_existing=True
+                replace_existing=True,
+                max_instances=1,
+                coalesce=True,
+                misfire_grace_time=30
             )

139-149: Remove duplicate datetime import and avoid hard-coded timeout

datetime is already imported at module top. Also prefer settings.http_timeout over a magic number.

-            import httpx
-            from datetime import datetime
+            import httpx
@@
-            async with httpx.AsyncClient(timeout=10.0) as client:
+            async with httpx.AsyncClient(timeout=settings.http_timeout) as client:

If you prefer, you can move import httpx to the module top as well for consistency.


160-161: Narrow exception handling for HTTP errors

Catch httpx.HTTPError to avoid masking unrelated bugs.

-        except Exception as e:
-            print(f"❌ 배치 정보 전송 중 오류: {e}")
+        except httpx.HTTPError as e:
+            print(f"❌ 배치 정보 전송 중 HTTP 오류: {e}")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1213f00 and bcab603.

📒 Files selected for processing (8)
  • services/painting-surface-data-simulator-service/Dockerfile (1 hunks)
  • services/painting-surface-data-simulator-service/app/config/settings.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/main.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/routers/test_connection_router.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/azure_storage.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/model_client.py (1 hunks)
  • services/painting-surface-data-simulator-service/app/services/scheduler_service.py (1 hunks)
  • services/painting-surface-data-simulator-service/requirements.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • services/painting-surface-data-simulator-service/requirements.txt
🧰 Additional context used
🧬 Code graph analysis (6)
services/painting-surface-data-simulator-service/app/services/azure_storage.py (2)
services/press-fault-data-simulator-service/app/services/azure_storage_service.py (2)
  • AzureStorageService (10-217)
  • __init__ (13-30)
services/press-fault-data-simulator-service/app/services/scheduler_service.py (1)
  • SchedulerService (12-208)
services/painting-surface-data-simulator-service/app/config/settings.py (3)
services/press-fault-data-simulator-service/app/config/settings.py (1)
  • Settings (7-47)
services/press-fault-data-simulator-service/app/services/scheduler_service.py (1)
  • SchedulerService (12-208)
services/press-fault-data-simulator-service/app/services/azure_storage_service.py (1)
  • __init__ (13-30)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (4)
services/welding-machine-data-simulator-service/app/services/azure_storage.py (1)
  • disconnect (29-32)
services/painting-surface-data-simulator-service/app/services/model_client.py (2)
  • health_check (90-103)
  • predict_painting_surface_data (18-63)
services/painting-surface-data-simulator-service/app/utils/logger.py (3)
  • log_anomaly (15-34)
  • log_normal_processing (36-39)
  • log_error (41-55)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • backend_service_url (39-41)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • backend_service_url (39-41)
services/painting-surface-data-simulator-service/app/main.py (3)
services/painting-surface-data-simulator-service/app/config/settings.py (1)
  • backend_service_url (39-41)
services/painting-surface-data-simulator-service/app/services/scheduler_service.py (2)
  • stop (51-60)
  • get_status (163-171)
services/painting-surface-data-simulator-service/app/services/model_client.py (1)
  • health_check (90-103)
services/painting-surface-data-simulator-service/app/routers/test_connection_router.py (2)
services/press-fault-data-simulator-service/tests/test_simulator_router.py (1)
  • router_test_client (47-51)
services/press-defect-detection-model-service/tests/test_predict_router.py (1)
  • app_and_client (45-54)
🪛 Ruff (0.12.2)
services/painting-surface-data-simulator-service/app/services/azure_storage.py

1-1: typing.Any imported but unused

Remove unused import: typing.Any

(F401)


2-2: asyncio imported but unused

Remove unused import: asyncio

(F401)


4-4: azure.core.exceptions.AzureError imported but unused

Remove unused import: azure.core.exceptions.AzureError

(F401)

services/painting-surface-data-simulator-service/app/config/settings.py

1-1: os imported but unused

Remove unused import: os

(F401)


2-2: typing.Dict imported but unused

Remove unused import: typing.Dict

(F401)

services/painting-surface-data-simulator-service/app/services/scheduler_service.py

44-44: f-string without any placeholders

Remove extraneous f prefix

(F541)


73-73: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/app/services/model_client.py

5-5: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)


21-21: f-string without any placeholders

Remove extraneous f prefix

(F541)


30-30: f-string without any placeholders

Remove extraneous f prefix

(F541)


77-80: Use ternary operator final_status = "anomaly" if defect_count > 0 else "normal" instead of if-else-block

Replace if-else-block with final_status = "anomaly" if defect_count > 0 else "normal"

(SIM108)


102-102: Do not use bare except

(E722)


171-171: f-string without any placeholders

Remove extraneous f prefix

(F541)


177-177: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/app/main.py

8-8: app.services.model_client.painting_surface_model_client imported but unused

Remove unused import: app.services.model_client.painting_surface_model_client

(F401)


45-45: f-string without any placeholders

Remove extraneous f prefix

(F541)

services/painting-surface-data-simulator-service/app/routers/test_connection_router.py

4-4: app.config.settings.settings imported but unused

Remove unused import: app.config.settings.settings

(F401)

🔇 Additional comments (4)
services/painting-surface-data-simulator-service/app/config/settings.py (1)

33-41: Property targets a defined field — LGTM.

model_service_url now correctly resolves to painting_model_url. No action needed.

services/painting-surface-data-simulator-service/app/main.py (1)

79-83: Fix connection status check (attribute doesn’t exist).

AzureStorageService here doesn’t expose client; the current check always reports “disconnected”. Report configuration status instead, or add a proper connectivity flag on the service.

         "azure_storage": {
             "container": settings.azure_container_name,
             "data_folder": settings.painting_data_folder,
-            "connection_status": "connected" if hasattr(azure_storage, 'client') and azure_storage.client else "disconnected"
+            # Report configuration status; for live connectivity, expose/consult a real flag or endpoint.
+            "connection_status": "configured" if bool(settings.azure_connection_string) else "not_configured"
         }

If you prefer a live check, introduce AzureStorageService.is_connected: bool and update during startup/shutdown.

Likely an incorrect or invalid review comment.

services/painting-surface-data-simulator-service/app/services/model_client.py (1)

18-63: Overall structure looks solid

Input validation, per-image error isolation, exponential backoff, and a clean combined summary are well put together. Nice.

services/painting-surface-data-simulator-service/app/services/scheduler_service.py (1)

15-46: Scheduler lifecycle and registration look good

Idempotent start/stop guards, health gate before scheduling, and clear operator logs are all on point.

Comment on lines +18 to +20
# 환경 설정 파일 복사
COPY .env ./

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Do not bake .env into the image (secrets exposure).

COPYing .env into the container risks leaking credentials (e.g., Azure keys) via image layers and registries. Pass env at runtime instead (docker run -e, Compose env_file, or orchestrator secrets).

-# 환경 설정 파일 복사
-COPY .env ./
+## 환경 변수/비밀은 런타임에 주입하세요 (Docker run/Compose/K8s Secrets 등)
+# ENV is set by the runtime; do not COPY .env into the image

Follow-ups:

  • Add a .dockerignore to exclude .env, logs/, __pycache__/, .pytest_cache/, etc.
  • Consider a non-root user and a slim base to reduce attack surface.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# 환경 설정 파일 복사
COPY .env ./
## 환경 변수/비밀은 런타임에 주입하세요 (Docker run/Compose/K8s Secrets 등)
# ENV is set by the runtime; do not COPY .env into the image
🤖 Prompt for AI Agents
In services/painting-surface-data-simulator-service/Dockerfile around lines
18-20, do not COPY the .env into the image because it bakes secrets into layers;
remove the "COPY .env ./" line and rely on passing environment variables at
runtime (docker run -e / docker-compose env_file / orchestrator secrets or
secret manager), add a .dockerignore to exclude .env, logs/, __pycache__/,
.pytest_cache/, etc., and as follow-ups consider switching to a slim base image
and creating a non-root user in the Dockerfile to reduce attack surface.

게이트웨이에 맞는 서버포트로 변경함에 따라 백엔드 url 변경
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (4)
services/painting-surface-data-simulator-service/app/config/settings.py (4)

1-3: Clean up imports; add ones needed for validation and lazy settings.

Drop unused os, Dict; add lru_cache, field_validator, SecretStr, numeric constrained types, and SettingsConfigDict.

-import os
-from typing import Dict
-from pydantic_settings import BaseSettings
+from functools import lru_cache
+from pydantic import field_validator, SecretStr, NonNegativeFloat, PositiveInt, NonNegativeInt
+from pydantic_settings import BaseSettings, SettingsConfigDict

6-13: Fail fast and mask secrets for AZURE_CONNECTION_STRING.

Validate non-empty and store as SecretStr to avoid accidental logging.

 class Settings(BaseSettings):
     # Azure Storage 설정
-    azure_connection_string: str
+    azure_connection_string: SecretStr

Add validator inside the class:

+    @field_validator("azure_connection_string")
+    @classmethod
+    def _validate_conn_str(cls, v: SecretStr):
+        if not v or not v.get_secret_value().strip():
+            raise ValueError("AZURE_CONNECTION_STRING is required and cannot be blank.")
+        return v

26-27: Don’t mix normal and error logs. Use a distinct error filename.

Prevents noisy, hard-to-filter logs.

-    error_log_filename: str = "painting_defect_detections.json"
+    error_log_filename: str = "painting_defect_errors.json"

49-49: Avoid import-time Settings instantiation; use a cached getter.

Prevents import failures when env isn’t set and aligns with testability.

-settings = Settings()
+@lru_cache(maxsize=1)
+def get_settings() -> "Settings":
+    return Settings()

Follow-up: replace call sites from app.config.settings import settings with:

from app.config.settings import get_settings
settings = get_settings()
🧹 Nitpick comments (3)
services/painting-surface-data-simulator-service/app/config/settings.py (3)

15-16: Constrain numeric config to prevent invalid values.

Allow 0+ minutes, require positive batch size.

-    scheduler_interval_minutes: float = 0.5
-    batch_size: int = 4
+    scheduler_interval_minutes: NonNegativeFloat = 0.5
+    batch_size: PositiveInt = 4

29-31: Constrain HTTP client parameters.

Timeout must be > 0; retries >= 0.

-    http_timeout: int = 30
-    max_retries: int = 3
+    http_timeout: PositiveInt = 30
+    max_retries: NonNegativeInt = 3

43-46: Use SettingsConfigDict and ignore unknown env vars (consistency with other service).

-    model_config = {
-        "env_file": ".env",
-        "env_file_encoding": "utf-8"
-    }
+    model_config = SettingsConfigDict(
+        env_file=".env",
+        env_file_encoding="utf-8",
+        extra="ignore",
+    )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bcab603 and d667b47.

📒 Files selected for processing (1)
  • services/painting-surface-data-simulator-service/app/config/settings.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
services/painting-surface-data-simulator-service/app/config/settings.py (3)
services/press-fault-data-simulator-service/app/config/settings.py (1)
  • Settings (7-47)
services/press-fault-data-simulator-service/tests/utils/test_logger.py (1)
  • fake_settings (50-74)
services/press-fault-data-simulator-service/app/services/scheduler_service.py (1)
  • SchedulerService (12-208)
🪛 Ruff (0.12.2)
services/painting-surface-data-simulator-service/app/config/settings.py

1-1: os imported but unused

Remove unused import: os

(F401)


2-2: typing.Dict imported but unused

Remove unused import: typing.Dict

(F401)

🔇 Additional comments (1)
services/painting-surface-data-simulator-service/app/config/settings.py (1)

33-41: Service URL accessors look good.

모델에 requirements.txt에 ultralytics 라이브러리 추가
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
services/painting-surface-defect-detection-model-service/requirements.txt (1)

4-4: Duplicate pin for ultralytics — remove one to avoid “Double requirement given” and resolver ambiguity.
You already pin ultralytics at Line 4; Line 13 repeats the same pin.

Apply this minimal diff:

- ultralytics==8.3.169

Also applies to: 13-13

🧹 Nitpick comments (2)
services/painting-surface-defect-detection-model-service/requirements.txt (2)

14-14: Consider dropping ultralytics-thop unless you actually use FLOPs/profiling at runtime.
If unused in serving paths, removing it reduces image size and attack surface. If needed only for dev, move it to test/dev requirements.


15-15: Compatibility Verified – Consider Documenting or Removing the Explicit urllib3 Pin

I ran your provided script under Python 3.11.2 and confirmed that:

  • Installing requests==2.31.0 alongside urllib3==2.5.0 produces no dependency conflicts (pip check reports no broken requirements).
  • At runtime, requests reports version 2.31.0 and urllib3 reports version 2.5.0 on Python 3.11.2.

Unless you’re pinning urllib3 to address a specific CVE or compatibility issue, you can safely remove this transitive pin to allow future urllib3 updates. If you do need to keep it, please add a brief comment in requirements.txt explaining the rationale (e.g. CVE reference, downstream compatibility bug, etc.) so that future maintainers understand why the override exists.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d667b47 and 130c40f.

📒 Files selected for processing (1)
  • services/painting-surface-defect-detection-model-service/requirements.txt (1 hunks)

requirements.txt에 중복된 라이브러리 제거
@TaeHyunaivle TaeHyunaivle merged commit 3bd81de into main Aug 27, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants